fix(mount): checkpoint incremental event pages#198
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
|
Reviewed against #197 — design and implementation satisfy what the issue asked for. Checklist vs #197
Optional, not implemented (was marked optional in #197)Per-cycle page cap. Not needed; the natural cycle interval + persisted resume cursor already give bounded cycle time + convergence. CI test failure is unrelated to this PR
This is a flaky test I authored in #196 — it computed Fixing the assertion (extract numeric event id, assert |
There was a problem hiding this comment.
🔴 Bootstrap full-pull path does not clear IncrementalBacklogDraining, causing perpetual incorrect 'syncing' status
Every code path in pullRemote that performs a full reset of sync state clears IncrementalBacklogDraining — the forceFullPull closure (syncer.go:2147), the no-events short-circuit (syncer.go:2152), the 404-fallback (syncer.go:2181), and the restart fast-path (syncer.go:2235). However, the main bootstrap full-pull path at syncer.go:2261 only clears IncrementalCheckpoint and omits the IncrementalBacklogDraining = false reset.
If the daemon loads persisted state with IncrementalBacklogDraining=true (from a prior interrupted incremental drain) and then enters the bootstrap path — most concretely when forceFullReconcile=true (via --full-reconcile or RELAYFILE_FORCE_FULL_RECONCILE=true) — the flag is never cleared. Because markSyncSuccess() at syncer.go:4207 gates LastSuccessfulReconcileAt on !IncrementalBacklogDraining, the timestamp is never updated, and the public state perpetually reports status: "syncing" even though complete full-tree pulls succeed every cycle. With forceFullReconcile=true, every cycle re-enters this same path, so the condition persists indefinitely.
(Refers to line 2261)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in b7618d8.
The main bootstrap/full-pull path now clears IncrementalBacklogDraining immediately after pullRemoteFull succeeds, matching the other reset paths. I also extended TestFullReconcileBypassesQuietEventsShortCircuit to persist a stale backlog-draining flag before a forced full reconcile and assert the flag clears and public state returns to ready.
Verification rerun:
go test ./internal/mountsync -run 'TestFullReconcileBypassesQuietEventsShortCircuit|TestPullRemoteIncremental(PersistsAppliedPageCursorOnListEventsError|ReturnsDeadlineWhenNoPageProgress|ResumesWithinAppliedPage|CheckpointPreservesChangedPath404Delete)'go test ./internal/mountsyncscripts/check-contract-surface.shgit diff --checkgo test ./...
Summary
Closes #197.
Verification