Back off checkpoint capture after repeated failures#4517
Conversation
On a repository large enough that a full-tree git add -A cannot finish inside the 30s VCS process timeout, capture is killed on every completed turn and retried on the next one. That pegs a CPU core for as long as any thread is in use, and each kill landing mid-pack-write leaves an orphaned tmp_pack_* in .git/objects/pack. Track consecutive capture failures per workspace and, after three, skip capture for a growing cooldown capped at an hour. Skipped captures replay the recorded failure so callers see the same error and the same missing status as before, without spawning git. Any success clears the record, so a transient failure costs nothing. Fixes pingdotgg#3646
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Threading an injectable clock through the service constructor reindented the whole factory for one test seam. The cooldown is minutes long, so the integration test stays inside it on the real clock, and the pure backoff tests already cover time-dependent transitions.
Only a later success clears a workspace record, so a workspace that fails once and is then abandoned or deleted would be retained, along with its error, for the lifetime of the server. Cap the tracked set and evict the least recently touched entries, which costs an evicted workspace nothing beyond its failure history.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review This PR introduces a new backoff mechanism that gates when checkpoint capture operations run, deciding to skip captures after repeated failures. Changes that control whether significant work executes warrant human review, regardless of how clean the implementation appears. You can customize Macroscope's approvability policy. Learn more. |
Two gaps in the backoff, both raised in review. Eviction recency was only refreshed when a failure was recorded, but a workspace inside its cooldown never records one. Churn from 256 other workspaces could evict a workspace that was actively being skipped and release its next capture early. Reads now refresh recency too. Threads sharing a workspace complete turns independently, so every one of them was released at once when a cooldown expired and they all launched the same capture together. The first caller past an expired cooldown now reserves the attempt. Reporting the outcome replaces the reservation with the real cooldown, and an unreported attempt lets it lapse on its own, so an interrupted capture cannot wedge the workspace.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b2c9ddb. Configure here.
Two follow-ups from review. A workspace under the failure threshold carries a zero deadline, which read as a cooldown that had just expired, so the first caller reserved the attempt and a concurrent capture was skipped after a single transient failure. Reserve only once the threshold is reached. Resolving the VCS driver sat outside the reported scope, so a failure before the capture ran left the caller's reservation in place with no recorded outcome. Resolve inside that scope so every failure path replaces the reservation with a real decision.
1 participant
What Changed
CheckpointStore.captureCheckpointtracks consecutive failures per workspace. After three, capture is skipped for a growing cooldown (5 minutes, doubling, capped at an hour) instead of being retried on every turn. Any success clears the record.Why
Checkpoint capture runs a full-tree
git add -Aunder a temporary index after every completed turn. On a repository large enough that this cannot finish inside the 30s VCS process timeout, the process is killed every single time, so the retry buys nothing and costs a great deal: one CPU core pegged for as long as any thread is in use, plus an orphanedtmp_pack_*in.git/objects/packfor every kill that lands mid-pack-write.A skipped capture replays the failure recorded when the cooldown opened, so callers see the same error and the same
missingturn-diff status they see today, and the error channel is unchanged. Three failures of tolerance keeps a transient failure (an index lock held by a concurrent git command) from disabling capture, and the hour cap means a workspace that becomes healthy is picked up again on its own.This is the "skip-with-backoff after N consecutive capture failures" option from the issue. The other suggestions there (a disable setting, an adaptive timeout,
tmp_pack_*cleanup) are deliberately left out to keep this focused.Fixes #3646
Testing
vp test run apps/server/src/checkpointing/passes (23/23). New coverage: the backoff policy itself (threshold, doubling, cap, per-workspace isolation, reset on success) and an end-to-end test drivingCheckpointStoreagainst a driver that always times out, asserting the driver is invoked exactly 3 times across 10 turns and exactly once more after the cooldown lapsesvp test run apps/server/src/orchestration/passes, 214 tests total across both directoriespnpm typecheckin apps/server cleanvp linton changed files cleanChecklist
Note
Medium Risk
Changes when checkpoints are captured for unhealthy workspaces (skipped turns still error the same way) but reduces runaway CPU/git load; logic is well-tested and scoped to capture only.
Overview
Adds per-workspace capture backoff so repeatedly failing checkpoint captures stop spawning expensive
git addwork on every turn.New
CaptureBackofftracks consecutive failures percwd(tolerates 2, then exponential cooldown from 5 minutes up to 1 hour, LRU cap of 256 workspaces, 60s single-flight reservation after cooldown).CheckpointStore.captureCheckpointconsultsbeginAttemptfirst; during cooldown it fails immediately with the same stored error instead of calling the VCS driver. Success clears state; failures (including driver resolve errors) update backoff and log warnings.Unit tests cover the policy; integration tests assert only three real capture attempts across many turns when capture always times out, plus recovery and registry-failure behavior.
Reviewed by Cursor Bugbot for commit 5a76e35. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Back off checkpoint capture in
CheckpointStoreafter repeated failuresCaptureBackoff, a per-workspace LRU-bounded tracker that computes exponential cooldown durations once a failure threshold is crossed, capping at an absolute maximum.CheckpointStore.captureCheckpoint: during cooldown, the method short-circuits and replays the last error without invoking the VCS driver; a single caller is reserved for the first post-cooldown retry.captureCheckpointcallers in cooldown now receive a replayed error immediately rather than triggering a new capture attempt, which is a behavioral change for any caller that retries on every error.Macroscope summarized 5a76e35.