[HEAVILY WIP] Migrate to ClickHouse#1470
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Greptile SummaryThis PR moves heartbeat storage and analytics from Postgres rollups to ClickHouse. The main changes are:
Confidence Score: 3/5This PR has a core account-merge bug that should be fixed before relying on the migrated path. The migration touches heartbeat ingestion, exports, dashboards, API stats, admin tools, and ClickHouse schema. The account-merge path can leave account merges partially applied across Postgres and ClickHouse. The PR is also marked heavily WIP, so changed heartbeat paths need careful validation before merge.
What T-Rex did
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant API as Hackatime API
participant Ingest as HeartbeatIngest
participant Writer as Clickhouse::HeartbeatWriter
participant CH as ClickHouse heartbeats
participant Stats as Stats/Dashboard services
Client->>API: POST heartbeats
API->>Ingest: normalize direct/import heartbeat payloads
Ingest->>Writer: insert_rows(rows)
Writer->>CH: INSERT native heartbeat rows
Stats->>CH: read deduped FINAL heartbeat scopes
CH-->>Stats: durations, aggregates, spans, exports
Stats-->>Client: API/dashboard/export responses
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant API as Hackatime API
participant Ingest as HeartbeatIngest
participant Writer as Clickhouse::HeartbeatWriter
participant CH as ClickHouse heartbeats
participant Stats as Stats/Dashboard services
Client->>API: POST heartbeats
API->>Ingest: normalize direct/import heartbeat payloads
Ingest->>Writer: insert_rows(rows)
Writer->>CH: INSERT native heartbeat rows
Stats->>CH: read deduped FINAL heartbeat scopes
CH-->>Stats: durations, aggregates, spans, exports
Stats-->>Client: API/dashboard/export responses
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
app/controllers/admin/account_merger_controller.rb:109
**Keep merge atomic**
`merge_user_heartbeats!` now runs after the Postgres transaction has committed and after `newer_user.destroy!`. If the ClickHouse insert or tombstone fails, the newer user and related Postgres rows are already deleted while their heartbeats remain under the deleted `user_id`, so the account merge is only partially applied despite the rescue claiming rollback.
Reviews (3): Last reviewed commit: "Numerous fixes" | Re-trigger Greptile |
|
Note to self: do we need |
|
@greptileai review |
|
@greptile-apps review |
| end | ||
| end | ||
| connection.execute(<<~SQL.squish) | ||
| INSERT INTO #{table} (#{column_list(connection)}) |
| column == "user_id" ? older_user_id.to_i.to_s : connection.quote_column_name(column) | ||
| end | ||
| connection.execute(<<~SQL.squish) | ||
| INSERT INTO #{table} (#{column_list(connection)}) |
| end | ||
| end | ||
| connection.execute(<<~SQL.squish) | ||
| INSERT INTO #{table} (#{column_list(connection)}) |
| results << "user ##{newer_user.id} deleted" | ||
| end | ||
|
|
||
| Clickhouse::HeartbeatWriter.merge_user_heartbeats!(older_user_id: older_user.id, newer_user_id: newer_user.id) |
There was a problem hiding this comment.
Keep merge atomic
merge_user_heartbeats! now runs after the Postgres transaction has committed and after newer_user.destroy!. If the ClickHouse insert or tombstone fails, the newer user and related Postgres rows are already deleted while their heartbeats remain under the deleted user_id, so the account merge is only partially applied despite the rescue claiming rollback.
Artifacts
Repro: runtime version check output
- Keeps the command output available without making the summary code-heavy.
Repro: Rails boot blocker from bundle check
- Keeps the command output available without making the summary code-heavy.
Repro: executable account merge atomicity harness
- Contains supporting evidence from the run (text/x-ruby; charset=utf-8).
Repro: harness output showing partial Postgres commit after ClickHouse failure
- Keeps the command output available without making the summary code-heavy.
Ran code and verified through T-Rex
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/controllers/admin/account_merger_controller.rb
Line: 109
Comment:
**Keep merge atomic**
`merge_user_heartbeats!` now runs after the Postgres transaction has committed and after `newer_user.destroy!`. If the ClickHouse insert or tombstone fails, the newer user and related Postgres rows are already deleted while their heartbeats remain under the deleted `user_id`, so the account merge is only partially applied despite the rescue claiming rollback.
How can I resolve this? If you propose a fix, please make it concise.
Warning
This PR is incomplete.
Summary of the problem
Describe your changes
Not in this PR but there's a PeerDB mirror from Postgres -> CH atm. It'll get turned off once we move
Screenshots / Media
N/A