feat(core): a configurable client lease, and a job on its last attempt survives a lost lease - #44
Merged
Merged
Conversation
…t survives a lost lease
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.
Summary
Two changes to what happens when a client cannot renew its lease, both for programs whose jobs are long calls to another system.
Config.LeaseTTL. The client lease was fixed at 15 seconds, renewed every 5. A database outage longer than that cancels every running job on the client, which is right for short jobs and expensive when a job is forty minutes into a cloud operation. The lease length is now a config field (default unchanged, minimum one second), renewed three times per TTL. It is per client, so each program picks its own; the rescuer only readsexpires_at, so clients with different leases can share an install. A longer lease delays the rescue of a crashed client's jobs by the same amount.A job on its last attempt is no longer cancelled when the lease is lost. Fencing cancels running jobs so that an attempt cannot overlap the one another client starts after the rescue. A job with no attempts left is never run again (the rescuer discards it), so there is nothing to overlap and the cancel only threw its work away. Such a job now runs under the work context, which only a hard stop cancels, instead of the lease generation. Jobs with attempts left behave as before. This is not a setting: I could not find a case where cancelling the last attempt is the better outcome.
One consequence, documented in the plan and the operations guide: if the leader's rescue lands before the job's own result, the job stays dead-lettered as
client lostalthough its work finished. Recording the late result would mean letting a finalize overwrite a rescue, which changes the finalize fence, and is left out of this change.Testing
TestFencedClientLetsALastAttemptFinish: a one-attempt job is running when its client's lease is expired behind its back; the client re-registers, the job's context is not cancelled and it runs once. With the change reverted the test fails (the last attempt was cancelled when the lease was lost).TestLeaseTTLSetsTheLeaseAndItsRenewaland a new validation case for a lease under a second.-raceagainst Postgres 18, andgolangci-lint, pass. The existing fencing test, which covers a job with attempts left, is unchanged and passes.No SQL changed, and nothing on the insert, claim or finalize paths, so no
hopperbenchrun.Checklist
make checkpasses (core module lint and all tests run locally)GODEBUG=fips140=only)docs/PLAN.md