Fix: unsatisfied-slot safety pause can live-lock and permanently over-count - #62
Open
sebclark wants to merge 1 commit into
Open
Fix: unsatisfied-slot safety pause can live-lock and permanently over-count#62sebclark wants to merge 1 commit into
sebclark wants to merge 1 commit into
Conversation
…table first-reserved anchor
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.
The bug (user-reported)
The safety pause counts
the tracker's reported unsatisfied count + local in-flight reservations. Reservations are retired byReconcileonce the provider's summary snapshot postdates the reservation's accounting window (ReservedUtc+ 20 min). But retrying an unconfirmed reservation deliberately refreshesReservedUtc(perretry_should_refresh_an_unconfirmed_reservations_accounting_window) — so a release that keeps failing and re-grabbing slides its window forward forever and its reservation becomes immortal:Restarts and the account-status refresh task don't help because the reservation rows persist in the database and the slide resumes. Symptom: the reported unsatisfied number sits exactly N above the tracker's own count, indefinitely.
Fix
Keep the window-refresh behaviour (the existing test still passes untouched) but add an immutable anchor and a hard ceiling:
FirstReservedUtc, backfilled fromReservedUtc??=before the slide)Reconcileunconditionally retires any reservation older than 24h from its first attempt, with an Info log naming the torrentA reservation that genuinely satisfies still retires via the existing snapshot rule long before the ceiling; the ceiling only catches the pathological retry loop.
Tests
reservation_should_retire_after_maximum_lifetime_despite_retries— just-retried row (snapshot rule can never cover it) with a first attempt beyond the ceiling → retiredreservation_within_maximum_lifetime_should_survive_a_stale_snapshot— young reservation is untouchedretry_should_preserve_the_first_reserved_anchor— the slide keeps refreshingReservedUtcbut the anchor holds (also backfills legacy rows on first retry)Note: no fails-first repro test for this one — the fix adds a schema column, so a pre-fix repro wouldn't compile. The live-lock derivation above plus the invariant tests are the evidence.