The retry sweep read the whole schedule to prune a handful of claims - #773
Merged
Merged
Conversation
The claim map is pruned against the schedule, and the read that pruned it asked for the whole live schedule. Every sweep materialized every scheduled record to decide the fate of a handful of claims, and the first sweep of all — holding no claims — paid for it too. After an outage that is the entire outage read once a minute, in the goroutine the stranded and loss repair share, to answer a question about at most a batch of ids. So the read is of the claims: StillScheduledBlockedIDs takes the ids held and returns the subset still scheduled, in chunks whose union is the answer, and the sweep skips it entirely when it holds nothing. Pruning now considers only the ids it asked about. That is the bound's safety, not an extra: a claim taken while the read was in flight was never put to the ledger, and dropping it for not coming back would offer its record a second time — which the old prune-everything-unnamed would have done. The answer was never wrong, which is why four rounds of review and a green suite never saw it. The assertion had to be on the work, so the ledger counts its schedule reads and the test asserts a sweep holding no claims makes none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approved
The implementation safely bounds schedule reads while preserving claim-pruning correctness and includes focused regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Optimizes blocked-retry pruning by querying only held claims while preventing concurrent claims from being incorrectly removed.
Changes:
- Adds bounded, chunked schedule lookups for claimed IDs.
- Prunes only claims included in the lookup snapshot.
- Adds regression and query-plan tests.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
internal/connector/ledger.go |
Adds schedule-read instrumentation. |
internal/connector/ledger_events.go |
Queries scheduled records by claimed IDs. |
internal/connector/intake.go |
Snapshots and safely prunes held claims. |
internal/connector/blocked_retry_test.go |
Covers bounded reads and safe pruning. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Once a minute, the blocked-retry sweep read the entire live schedule to decide the fate of a handful of claims it was holding. The first sweep of every run paid that cost while holding no claims at all, and after an outage — when the schedule is at its longest — that read is the whole outage.
The sweep now asks only about the claims it holds, and the ledger answers with the subset of those still scheduled. When it holds nothing, it asks nothing.
Bounding the question closed a second defect that four review rounds had missed. The old pruning dropped every claim the answer did not name — so a claim taken while the read was in flight, which was never put to the ledger and therefore could never come back in its answer, was dropped, and its record offered a second time. Pruning now considers only the ids it asked about.
Originally tracked in A blocked record's timed retry is described but never runs.
Follows up on A blocked record's timed retry runs. This commit was written after that PR's checks had gone green and missed its squash merge, so it lands on its own rather than as part of it.