fix(e2e): stabilize mv_replay empty-target backfill on ObsessionDB - #225
Conversation
select_sequential_consistency does not fetch parts from a non-quorum insert, so a chunk INSERT…SELECT on a stale replica finishes with 0 written rows and the dedup token makes that replay stick. Sync each active replica before reading, disable parallel replicas on the chunk query, and reissue any still-empty chunk under a new token. Co-authored-by: Marc Höffl <marc.hoeffl@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64939e67a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const [row] = await session.query<{ cnt: string }>( | ||
| `SELECT toString(count()) AS cnt FROM ${sourceFqn} SETTINGS select_sequential_consistency = 1`, | ||
| ) | ||
| if (Number(row?.cnt ?? 0) === SOURCE_ROWS) readySamples += 1 |
There was a problem hiding this comment.
When the load balancer routes multiple fresh sessions to the same healthy replica, each successful count increments readySamples, so this can reach active_replicas without ever checking a lagging replica. A subsequent stateless chunk can still land on that unchecked replica and reproduce the 0-row replay this change is intended to prevent. Record a replica identity with each sample and require the expected number of distinct ready replicas, or target every replica explicitly.
Useful? React with 👍 / 👎.
ObsessionDB's managed role cannot SELECT system.replicas, so the replica count added in #225 failed the empty-target e2e with ACCESS_DENIED. Fall back to one visibility sample, skip SYSTEM SYNC REPLICA only when that command is denied too, and keep parallel-replica disable plus empty-chunk replay. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
2 participants
Summary
The
obsessiondbjob on7564cac(run 36437314070) failed@chkit/plugin-backfill#testinpackages/plugin-backfill/src/mv-replay-plan.e2e.test.ts.verifywas green. One chunk'sINSERT…SELECTfinished withwrittenRows: 0(f26814ae900eeac9), so bucket"2"never landed in the empty aggregate. The other three chunks wrote one row each, and a consistent read of the source still had all four buckets (0–3).#221appendedselect_sequential_consistency = 1to the chunk SQL and polled the target. That poll cannot repair a chunk that already committed an empty replay. The setting does not fetch parts from a normal, non-quorum insert; it only hides or rejects blocks the quorum has not confirmed. Chunk queries are submitted on a stateless client, so each one can run on a different replica than the session that inserted the source. ObsessionDB also enables parallel replicas by default, and a stale follower can satisfy one partition with an empty scan while the query still reports finished. The chunk's dedup token then makes another attempt with the same plan id commit nothing again.The harness change is limited to that e2e:
SYSTEM SYNC REPLICA … LIGHTWEIGHTand then counts. The backfill does not start until every sample sees all 4000 source rows. Plain MergeTree (the single-nodeverifyservice) only counts.enable_parallel_replicas = 0, matching planning.Test plan
biome linton the test file, and the non-e2e@chkit/plugin-backfillunit testsverifycovers the single-node ClickHouse 26.3 path used in CIobsessiondbjob runs only on pushes tomain, so this PR's CI will not prove the live fix. After merge, watch theobsessiondbjob onmain. Do not treat a green PRverifyas coverage of this race.