Repository navigation
Conversation
…py pass The pass compares only through the watermark, so a shadow row above it holding a unique value the source has since moved into a differing chunk made every recopy of every reconcile collide; only --restart escaped.
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. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
🤖 Review on Jash’s behalf at Verdict: no concrete blocker found in the full diff, with the verification limits below. Clearing the shadow above the persisted watermark before comparing the retained prefix addresses the collision that repeated prefix-only comparisons cannot reach. Findings and action items
Verified and limitationsStatic full-diff review: clear errors return before the verifier runs; an invalid watermark still skips the pass; the wrapper binds work to the lock context and delegates to the existing clear implementation; options carry the pass's timeouts, chunker and clock. The added test checks zero collisions, repair coverage, unchanged watermark/phase, absence of rows above it, and final convergence after copy resumes. No existing tests were removed. GitHub reports 16 successful checks, including PostgreSQL 14–18. Exact-head surrounding-source retrieval failed ( Generated by Codex. |
|
🤖 Product review at OSS adoption: recovery now handles the documented partial-watermark collision without requiring an operator to discard the run with SchemaBot integration: the fix is inside Generated by Codex. |
JashLal
left a comment
There was a problem hiding this comment.
🤖 Approving on Jash’s behalf after the full-diff reviews at 4d8c1d6. No concrete blocker found; the optional interrupted-clear regression and verification limits are recorded above. No local build or tests ran. Generated by Codex.
Reconcile mode now clears the shadow above a partial watermark before its recopy pass, so a shadow row no comparison can reach never makes the recopy collide.
Why
A reconcile on resume compares the source with the shadow only through the row's watermark, but the shadow can hold rows above it: chunks the earlier run committed but never checkpointed before its slot was lost. Those rows are outside every comparison the pass makes and yet still hold their values. If the source has since moved a unique value out of one of them into a chunk below the watermark that the pass finds differing, the recopy of that chunk inserts the value and collides with the row above; comparing again cannot clear it, because that row is never compared. Every attempt of every reconcile collided and the run stayed in
reconcilinguntil the operator ran--restart. The copier that resumes from the watermark already clears the shadow above it first (CO-4); the reconcile ran before that clear.What
copier.ClearAboveexports the clear a resumedRunperforms before its first chunk — same batches, same SHARE ROW EXCLUSIVE fence on the first batch, bound to the table lock session (LK-1). A complete watermark clears nothing.Reconciler.recopycalls it beforeverifier.Checkwhenever the watermark says anything landed; nothing landed still skips both. The clear runs under options derived from the pass'schecksum.Options(timeouts, chunk size, clock), validated when theReconcileris built.TestReconcileClearsTheShadowAboveAPartialWatermarkBeforeItsRecopyfails onmainwith five23505collisions and passes here with none.docs/invariants.mdST-4 anddocs/copy-and-swap-design.mdD14 drop the Known limit and describe the clear.Before / after
The example is the regression test: 500 rows copied, the row checkpointed in
copyingat watermark 300 when the slot was lost, so the shadow holds keys 301–500 that the row never recorded. While the slot is gone the source changes key 150 (chunk 101–200 now differs) and moves key 400's unique value to key 160, in that same chunk. The only thing that changes is the clear before the pass; the pass itself, the recopy policy, and the phase the row returns to are unchanged.🤖 Generated with Amp (Claude Opus 4.6)