Repository navigation
fix: let the parsed time decide when claim_next finds a lease lost - #76
Merged
Merged
Conversation
`claim_next` selected the claimed tasks whose `lease_until` was before the present time, and compared the two times as text. A time renders with as many digits of a second as it has. A lease that ended at `10:05:00Z` was thus not lost at `10:05:00.5Z`, and `claim_next` gave no task for up to one second. The named `claim` gave the task at the same time, because it parses the times. The error was on the safe side: a task never had two holders. But a lost lease did not go back to the pool at its time. The two backends now select each open, blocked and claimed task. `claimable` decides with the parsed times, as it did after the selection. The adversarial pass on the taint change found this. The new assertion failed on each backend before the change: `called Option::unwrap() on a None value`, where `claim_next` gave no task. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…holder The adversarial pass ran on 8ed964c and confirmed two findings. - On PostgreSQL, `claim_next` reads the queue and then locks one row. A different connection can claim that task and commit between the two. The check after the lock then stops a second holder. On `main`, the time term in the query was a second guard, and 8ed964c removed it. With the check off, all ledger tests passed. The check is now in a function of its own, `take`, and a test gives it a task that a different holder claimed. - The queue was in the order of `created` as text. Of two tasks of one priority, filed at `10:00:00Z` and `10:00:00.5Z`, `claim_next` gave the second one first. The two backends now put the candidates in order by the parsed time. New assertions that failed before their fix: - the queue order: the pass ran it on 8ed964c, and `claim_next` gave "Late" on the two backends; - the check after the lock, with the check off: `assertion failed: !db.take_alone(..)`. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
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.
claim_nextselected the claimed tasks whoselease_untilwas before the present time, and compared the two times as text. A time renders with as many digits of a second as it has. A lease that ended at10:05:00Zwas thus not lost at10:05:00.5Z, andclaim_nextgave no task for up to one second. The namedclaimgave the task at the same time, because it parses the times.The error was on the safe side. A task never had two holders. But a lost lease did not go back to the pool at its time.
What changes
claimabledecides with the parsed times, as it did after the selection.created.Checks
make checkpassed. The new test runs on SQLite and on the embedded PostgreSQL.The new assertion failed on each backend before the change:
called Option::unwrap() on a None value, whereclaim_nextgave no task.Adversarial pass
The pass ran on 8ed964c and confirmed two findings. The second commit corrects them:
claim_nextreads the queue and then locks one row. A different connection can claim that task between the two. The check after the lock then stops a second holder. Onmain, the time term in the query was a second guard, and 8ed964c removed it. With the check off, all ledger tests passed. The check is now in a function,take, and a test gives it a task that a different holder claimed.createdas text. Of two tasks of one priority, filed a half second apart,claim_nextgave the second one first. The two backends now put the candidates in order by the parsed time.The pass confirmed that the limit did not change: at the time
lease_until, the lease is live.New assertions that failed before their fix:
claim_nextgave "Late" on the two backends;assertion failed: !db.take_alone(..).No pass read the second commit.
Open
The pass on #75 listed four more places that order time text: the day ceiling of the egress telemetry, the tape, the newest platform artifact, and the order of the task lists by
created. It read them and did not run them. Each is wrong only when two times are less than 10^-k seconds apart.Posted by Claude on behalf of @sstriker
🤖 Generated with Claude Code