Repository navigation
feat(jobs): single-runner leader gate for the durable job worker (#271) - #555
Conversation
Gate DurableJobWorker's poll and the three recurring sweeps behind a Postgres session-scoped advisory lock (pg_try_advisory_lock(271,1)) on a dedicated non-pooled connection, so at most one API instance is the leader under multi-replica operation. Followers keep their loop alive (health green) and re-attempt acquisition; crash recovery is automatic via session death releasing the lock. Contract is at-most-one-leader, not exactly-once: durable jobs are at-least-once with idempotent handlers. FOR UPDATE SKIP LOCKED job claiming is deferred (no handlers exist yet; the live double-run is the sweeps, which the gate covers). A faulted acquisition (DB unreachable) is distinct from a follower: it backs off with the heartbeat unstamped so /health degrades (#69), rather than reading as a healthy follower. Slice S1 of #530. Closes #271.
|
@codex review this PR. Focus areas: the Postgres advisory-lock leader gate (PostgresLeaderLease) — can two instances ever both be leader (session-liveness re-check, lock stacking, held→ping→reset path)? Does any failure mode crash the host instead of degrading /health? The tri-state Leader/Follower/Faulted routing in DurableJobWorker. Contract is at-most-one-leader, not exactly-once (by design). FOR UPDATE SKIP LOCKED job-claim is deferred deliberately (no handlers exist). Every test is mutation-verified. |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a056a80e6a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the app could not connect at all. | ||
| this.connectionString = new NpgsqlConnectionStringBuilder(connectionString) | ||
| { | ||
| Pooling = false, |
There was a problem hiding this comment.
When ConnectionStrings:Default points to a transaction-pooled proxy such as PgBouncer, Pooling = false only disables Npgsql's client pool; it does not pin commands to one PostgreSQL backend. pg_try_advisory_lock can therefore remain on a backend returned to the proxy pool while this object keeps held = true, and the later SELECT 1 may execute on another backend, allowing multiple replicas to run as leaders. Require a direct/session-pooled endpoint for this lease—potentially via separate portable configuration—or use a mechanism compatible with transaction pooling.
AGENTS.md reference: AGENTS.md:L126-L130
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in d05acda. The lease now captures the backend PID that took the advisory lock in the SAME statement as the acquire (SELECT pg_try_advisory_lock(@class,@obj), pg_backend_pid()), and re-verifies it on every poll: if the serving backend PID no longer matches (a transaction-pooling proxy multiplexing the connection onto a different backend), it relinquishes leadership rather than trust the stale held flag. Net: fail-safe on every topology — never a silent double-run; under transaction pooling no instance leads stably and /health degrades (visible). New test LeaderMigratedToAnotherBackend_RelinquishesTheStaleSession proves it via an injected backend-PID probe; mutation-verified (disabling the PID-equality check turns it red). Making the lease WORK (not just fail safe) under a pooled deploy — a dedicated session-pinned lease endpoint — is tracked as #556. Full integration suite green (1251).
There was a problem hiding this comment.
Correction to my earlier reply on this thread: I overstated it as 'fail-safe on every topology — never a silent double-run.' That's wrong under transaction pooling (see the follow-up P1). The affinity guard only narrows the window; single-leader holds only on a session-pinned endpoint. Corrected in 4077f1a; real fix tracked as #556.
A session advisory lock on a connection through a transaction-pooling proxy (e.g. PgBouncer txn mode) can land on a different backend than later commands, and the pooled lock-holding backend could be handed to another replica whose re-entrant pg_try_advisory_lock also succeeds — two leaders. Capture the backend PID that took the lock in the same statement as the acquire, and relinquish leadership on any poll where the serving backend PID no longer matches. Net: safe on every topology (never a silent double-run; degrades visibly under transaction pooling). Making it WORK under a pooled deploy (a dedicated session-pinned lease endpoint) is tracked as #556. Codex P1 on PR #555.
|
@codex the transaction-pooling P1 is addressed in d05acda — the lease captures the lock's backend PID in the same statement as the acquire and relinquishes leadership on any poll served by a different backend (fail-safe, no silent double-run). Follow-up #556 tracks a dedicated session-pinned lease endpoint for pooled deploys. Please re-review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d05acda9c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var (acquired, backendPid) = await TryLockAsync(connection, ct); | ||
| held = acquired; | ||
| if (acquired) | ||
| lockBackendPid = backendPid; | ||
| return held ? LeaseStatus.Leader : LeaseStatus.Follower; |
There was a problem hiding this comment.
When ConnectionStrings:Default uses transaction pooling, the implicit transaction containing this acquisition ends before the method returns, so the proxy can immediately assign the lock-owning backend to another replica; because session advisory locks are reentrant, that replica's pg_try_advisory_lock can also succeed and both workers can enter ProcessPendingJobsAsync before either performs its next PID probe. Fresh evidence in d05acda is that the acquisition path still returns Leader immediately without a post-acquisition affinity check covering the ensuing work, so the later mismatch detection does not make this topology fail-safe. Require a session-pinned endpoint or another transaction-pool-compatible lease before returning leadership.
AGENTS.md reference: AGENTS.md:L126-L130
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
You're right, and thanks for pushing on it. The affinity check runs at the START of each poll, not on the acquire path, so a second replica can reuse the pooled lock-holding backend and run one ProcessPendingJobsAsync cycle concurrently before either's next probe — the guard NARROWS but does not close the transaction-pooling window. There is no cheap client-side full fix (pinning the backend would mean an indefinitely-open transaction — idle-in-transaction, vacuum starvation, worse). So I've corrected the overclaim in 4077f1a: single-leader is guaranteed only on a SESSION-PINNED endpoint; under transaction pooling safety rests on the at-least-once + idempotent-sweep contract (which the sweeps satisfy), NOT on single-leader, and /health degrades. The real fix — a dedicated session-pinned lease endpoint so pooled deploys are single-leader-safe — is #556. Not claiming fail-safe under pooling any more.
| // #271 — the single-runner gate the worker acquires before polling. | ||
| services.AddSingleton<ILeaderLease>(sp => new PostgresLeaderLease( | ||
| sp.GetRequiredService<LeaderLeaseConnectionString>().Value, | ||
| sp.GetRequiredService<ILogger<PostgresLeaderLease>>())); |
There was a problem hiding this comment.
This registration introduces the advisory-lock lease, but the canonical deployment guidance and its linked decision still state that the worker poll has no lease or advisory lock and list that absence as the #271 blocker. Leaving those architecture records unchanged makes subsequent scaling audits reason from behavior that this PR explicitly replaces; update them in the same change to describe the new lease and its transaction-pooling limitation while retaining the remaining single-instance blockers.
AGENTS.md reference: AGENTS.md:L126-L130
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed the AGENTS.md deploy-invariant + decision records go stale once this merges. That edit is deliberately owned by #537 (slice T9 — 'Docs: multi-farm tenancy ADR + AGENTS.md / GLOSSARY sync'), which lands alongside whichever slice settles each claim: the file explicitly warns that blocker list 'was derived wrong twice,' so partial edits from a feature PR are exactly what that split avoids. And AGENTS.md still correctly describes main until #555 merges. I've flagged the specific #271 'no lease' wording + the transaction-pooling limitation on #537 so it's captured there.
There was a problem hiding this comment.
Update: on reflection the #271-specific piece of this is a settled doc change, so I folded it into this PR (81c0c57) rather than leaving all of it for #537. The canonical AGENTS.md deploy-invariant paragraph and the 271 decision doc now describe the advisory-lock leader gate, its session-pinned requirement + transaction-pooling limitation (#556), and the re-derived remaining blockers (#143, #311) — the blocker list re-derived by walking every AddSingleton/AddHostedService under src/, not edited from memory (that list 'was derived wrong twice'). The broader multi-slice AGENTS.md/GLOSSARY/ADR reconciliation (tenancy, login, suspension — most blocked on unlanded slices) stays #537.
The affinity check narrows but does NOT close the transaction-pooling double-run window: it runs at the start of each poll, not on the acquire path, so a second replica can reuse the pooled lock-holding backend and run one poll cycle concurrently before the next probe. Single-leader is guaranteed only on a session-pinned endpoint; under transaction pooling safety rests on the at-least-once + idempotent-handler contract, not on single-leader. Corrects the earlier 'fail-safe on every topology' overclaim. Real fix (session-pinned lease endpoint) tracked as #556. Codex P1 on #555.
|
Review loop stopped deliberately here (not because Codex went silent). Two product defects were found and fixed across the rounds: the Faulted-vs-Follower health regression, and the transaction-pooling overclaim — corrected to an honest, documented limitation (single-leader only on a session-pinned endpoint; pooling relies on the at-least-once + idempotent-sweep contract). The remaining Codex P1 asks for a session-pinned endpoint 'before returning leadership'; that is the real fix for pooled deploys and is deliberately deferred to #556 (owner decision), not a defect in the shipped single-runner guarantee for the reference topology. The AGENTS.md deploy-invariant sync is owned by #537 (T9) and flagged there. Not re-tagging @codex, since the next round would only re-raise the settled #556 deferral. |
|
No follow-up code change is needed. The trigger records an intentional review stop and an owner-approved deferral of transaction-pooled single-leader support to #556. The current implementation already distinguishes acquisition faults from healthy followers and resets the connection so health can degrade appropriately. src/Cluckwork.Infrastructure/Jobs/PostgresLeaderLease.csL151-L165 It also captures and checks backend PID affinity while documenting the transaction-pooling migration limitation. src/Cluckwork.Infrastructure/Jobs/PostgresLeaderLease.csL168-L178 src/Cluckwork.Infrastructure/Jobs/PostgresLeaderLease.csL202-L214 The working tree is clean, so no commit was created and—per repository instructions—no new pull request was opened. Checks
|
The worker no longer polls without a lease — AGENTS.md's deploy-invariant and the 271 decision doc said 'no lease / no advisory lock' and listed #271 as an open blocker. Update both to describe the advisory-lock leader gate, its session-pinned requirement + transaction-pooling limitation (#556), and the remaining single-instance blockers (#143, #311), re-derived by walking every AddSingleton/AddHostedService under src/ (not from memory). Broader multi-slice AGENTS.md/GLOSSARY reconciliation stays #537.
🤖 I have created a release *beep* *boop* --- ## [0.1.0](v0.0.4...v0.1.0) (2026-09-12) ### ⚠ BREAKING CHANGES * log in by farm code, with per-account email identity ([#532](#532)) (#564) ### Features * **accounts:** add Account.Slug (farm code), suspend/reactivate, list-accounts verb ([#531](#531)) ([3fe9754](3fe9754)) * **accounts:** provision additional farms ([#581](#581)) ([006f298](006f298)) * add Aspire local development AppHost ([#567](#567)) ([2c9e6b9](2c9e6b9)) * add configurable worker sale allocation ([#619](#619)) ([0955095](0955095)) * add searchable entity pickers ([#642](#642)) ([60d2053](60d2053)) * **api:** provision-account takes an optional --timezone at creation ([#603](#603)) ([#694](#694)) ([a0aee39](a0aee39)) * **audit:** show the sales-line audit payload as a readable Details column ([#745](#745)) ([#749](#749)) ([d26d389](d26d389)) * **auth:** add ApplicationUser.StepUpLogoutEpoch column ([#338](#338)) ([#554](#554)) ([18306ee](18306ee)) * certify over-cap simulation fixture bands ([#633](#633)) ([a67b2e1](a67b2e1)), closes [#627](#627) * **cli:** rename-account verb to change a farm code ([#732](#732)) ([#733](#733)) ([4b70559](4b70559)) * **customers:** edit existing customer details ([#625](#625)) ([#626](#626)) ([062a55c](062a55c)) * **jobs:** single-runner leader gate for the durable job worker ([#271](#271)) ([#555](#555)) ([4148f9b](4148f9b)) * let owners change user email addresses ([#605](#605)) ([842347b](842347b)) * log in by farm code, with per-account email identity ([#532](#532)) ([#564](#564)) ([68adb62](68adb62)) * **ratelimit:** distributed IP-keyed auth limiters ([#544](#544)) ([#558](#558)) ([ec14972](ec14972)) * **ratelimit:** distributed per-account report concurrency cap with local-ceiling fallback ([#545](#545)) ([#559](#559)) ([1522e4e](1522e4e)) * **sales:** mark discounted lines, total the discount, and show it in the Orders list ([#723](#723), [#724](#724)) ([#741](#741)) ([1a07441](1a07441)) * **sales:** record list, old and new price in the order-line audit payload ([#722](#722)) ([#742](#742)) ([97c866f](97c866f)) * **sales:** refuse an over-ceiling confirm from a Sales user ([#727](#727)) ([#766](#766)) ([8c0792a](8c0792a)) * **sales:** show what each order still owes, and filter the list to unpaid ([#771](#771)) ([ca59d68](ca59d68)) * **sales:** snapshot the list price on the order line and show the discount ([#734](#734)) ([cffed5e](cffed5e)) * **sales:** snapshot the product name and unit in the order-line audit payload ([#747](#747)) ([#748](#748)) ([0481c06](0481c06)) * scope Worker reads to assigned flocks ([#388](#388)) ([#611](#611)) ([5884a9a](5884a9a)) * shared-state ports with Redis + in-process fallback ([#543](#543)) ([#552](#552)) ([f767fa9](f767fa9)) * suspend-account / reactivate-account operator verbs ([#534](#534)) ([#573](#573)) ([d0be26c](d0be26c)) * **tenancy:** write-side tenant guard + single-assignment TenantContext ([#546](#546)) ([#561](#561)) ([f371f1d](f371f1d)) * **web:** dashboard rework — capture-status tiles, 14-day trend, stock as a stacked bar ([#654](#654)) ([396ba23](396ba23)) * **web:** date-range filters on audit and expenses, and the stock lot filter gets its bounded toolbar ([#666](#666), [#667](#667), [#653](#653)) ([94b188f](94b188f)) * **web:** elevation hierarchy and sentence-case labels ([#651](#651), [#652](#652)) ([#661](#661)) ([28db4c7](28db4c7)) * **web:** Expenses and Audit keep a clear-filters control while rows are still showing ([#679](#679)) ([#697](#697)) ([b859982](b859982)) * **web:** expenses filters by a date range like its sibling screens ([#667](#667)) ([f13858f](f13858f)) * **web:** key the farm brand palette per farm ([#586](#586)) ([#600](#600)) ([7183a43](7183a43)) * **web:** let operators forget remembered farms ([#598](#598)) ([577d94e](577d94e)) * **web:** one-line provenance, bounded date filters, and empty states that invite action ([#653](#653), [#655](#655)) ([#668](#668)) ([80b53f4](80b53f4)) * **web:** prefill the farm code from ?farm= and remember it ([#535](#535)) ([#588](#588)) ([b7f5cc6](b7f5cc6)) * **web:** split authenticated routes into lazy chunks ([#620](#620)) ([5089271](5089271)) * **web:** the audit log filters by a date range, and says which window is empty ([#666](#666)) ([63027e0](63027e0)) * **web:** typeset numbers as numbers and refresh the Help glossary ([#650](#650), [#657](#657)) ([af4fe11](af4fe11)) ### Bug fixes * **api:** order same-instant audit events by a durable monotonic key ([#700](#700)) ([8fcf084](8fcf084)) * **api:** print the farm code from bootstrap-admin ([#589](#589)) ([#594](#594)) ([34032ac](34032ac)) * **audit:** show the price a line sold for, not its list price ([#759](#759)) ([e6b37d0](e6b37d0)) * **audit:** store catalog enums by name and guard the add-item transaction shape ([#751](#751)) ([23609ff](23609ff)) * **auth:** reject invalid account claims ([#622](#622)) ([8d6c7fe](8d6c7fe)) * **auth:** require step-up for durable user access ([#360](#360)) ([#607](#607)) ([f767dce](f767dce)) * **ci:** bound the npm audit calls and give the web job room to finish ([#686](#686)) ([153b7a8](153b7a8)) * **ci:** escalate the audit bound to SIGKILL, so it actually bounds ([#686](#686)) ([a0c8f4e](a0c8f4e)) * **ci:** fail closed on invalid vulnerability config ([#621](#621)) ([1690db8](1690db8)) * **ci:** lockfix covers the two AppHost lock files, derived from the sln ([efb05e6](efb05e6)) * **ci:** lockfix covers the two AppHost lock files, derived from the sln ([8986d77](8986d77)) * **ci:** remove invalid XML comment from nuget.lockfix.config ([#541](#541)) ([5f1bc0a](5f1bc0a)) * **ci:** the advisory vuln gate no longer blocks on an unusable report ([#686](#686)) ([aaf6934](aaf6934)) * **ci:** the advisory vuln gate no longer blocks on an unusable report ([#686](#686)) ([64f1f53](64f1f53)) * **i18n:** tl help text names the saleable flag and unit-system setting what their labels call them ([#688](#688)) ([#696](#696)) ([bfd24d7](bfd24d7)) * **infra:** AccountId must be a non-nullable Guid or both tenant write layers refuse ([#673](#673)) ([#695](#695)) ([2470c4e](2470c4e)) * require step-up for flock scope changes ([#609](#609)) ([4151f89](4151f89)) * **sales:** keep a line's discount markers agreeing while its price is edited ([#752](#752)) ([#753](#753)) ([c159b4b](c159b4b)) * **sales:** say which kind of missing list price a line has ([#774](#774)) ([489180e](489180e)) * scope legacy logout to selected farm ([#624](#624)) ([fae8d82](fae8d82)) * **seed:** drain the daily-entry lock sweep so deep simulation fixtures validate ([#644](#644)) ([730fa23](730fa23)), closes [#638](#638) * **tenancy:** AccountId is a concurrency token, so the database refuses a detached cross-tenant write ([#562](#562)) ([4d1dfa3](4d1dfa3)) * **tenancy:** AspNetUserRoles carries a tenant column, so a role write naming another farm's user is refused ([#670](#670)) ([fc0552a](fc0552a)) * **tests:** bump the image-pin allow-list counts for the AppHost LocalPorts tests ([#593](#593)) ([58d3056](58d3056)) * **tests:** the OTLP collector survives a lost port race and ignores traffic that is not an export ([#672](#672), [#676](#676)) ([#677](#677)) ([965c737](965c737)) * **web:** a scoped audit view filtered to nothing names both the record and the range ([#666](#666)) ([41bbfe1](41bbfe1)) * **web:** an abandoned dialog attempt's success no longer hijacks the replacement on Customers, Daily Entry, Flocks, Grades and Products ([#703](#703)) ([#705](#705)) ([85605db](85605db)) * **web:** an abandoned dialog attempt's success no longer hijacks the replacement on Inventory, Expenses, History and Stock ([#703](#703)) ([#706](#706)) ([60a4997](60a4997)) * **web:** an abandoned edit's success no longer hijacks the dialog that replaced it on Users ([#703](#703)) ([#710](#710)) ([778faab](778faab)) * **web:** an abandoned order attempt's success no longer hijacks the dialog that replaced it ([#702](#702)) ([522c699](522c699)) * **web:** capture screens open on the flock you last used, and assigning one no longer guesses ([#646](#646)) ([#699](#699)) ([7f8f317](7f8f317)) * **web:** constrain dialog session helpers to declared scopes ([#715](#715)) ([389e3c8](389e3c8)) * **web:** date validation gets one boundary table instead of one case per review round ([#666](#666)) ([215f830](215f830)) * **web:** keep a paged window and an item panel on the user's newest intent ([#645](#645)) ([d81bccf](d81bccf)) * **web:** keep Sales order panels closed after pending writes ([#711](#711)) ([f0f7492](f0f7492)) * **web:** keep Sales panels closed after pending Open reads ([#716](#716)) ([620411f](620411f)) * **web:** make login take the cross-tab cookie lock so a racing refresh cannot restore the wrong session ([#648](#648)) ([ff18beb](ff18beb)) * **web:** make the entity picker read as a search field and focus it on open ([#736](#736)) ([66ef667](66ef667)), closes [#735](#735) * **web:** page truncated customer and movement tables with usePagedList ([7cfe4d6](7cfe4d6)) * **web:** reconcile Sales line edits with refreshed orders ([#717](#717)) ([d7dd2c9](d7dd2c9)) * **web:** the audit date filter accepts low-numbered years, and its empty state covers every narrowing ([#666](#666)) ([af52d25](af52d25)) * **web:** the audit date filter rejects impossible dates, and its history guard actually guards ([#666](#666)) ([8d51846](8d51846)) * **web:** the expense range bounds are not capped at today, which the month-end default exceeds ([#667](#667)) ([7e01864](7e01864)) * **web:** the help text calls the expiry field what the field calls itself ([#666](#666)) ([2fd1f3c](2fd1f3c)) * **web:** the stock lot date range sits in the bounded toolbar ([#653](#653)) ([43dec5e](43dec5e)) ### Refactoring * **web:** extract SalesPage's dialog-write wrapper into a shared useDialogAction hook ([#703](#703)) ([#704](#704)) ([60ee9d9](60ee9d9)) ### Documentation * add k6 preparation steps to the dev-database fixture runbook ([#643](#643)) ([a4f1f09](a4f1f09)) * add runbook for loading the simulation fixture into a dev database ([#639](#639)) ([2d143b8](2d143b8)) * **agents:** a PR closes its issue from the body, not the title ([#744](#744)) ([39be13c](39be13c)) * **agents:** drop the commit and push gate, and require screenshots on UI changes ([#757](#757)) ([6225172](6225172)) * **agents:** find guards by grepping registry readers; amend issues a PR overtakes ([#580](#580)) ([fe3fde8](fe3fde8)) * **agents:** the Playwright specs have been in CI since 2026-08-08 ([#768](#768)) ([68ee612](68ee612)) * **aspire:** record the second local database and pin the AppHost dashboard ports ([#623](#623)) ([713b941](713b941)) * compress AGENTS.md to one paragraph per rule, and draw the two orders that matter ([#551](#551)) ([997ae8a](997ae8a)) * item 7 names each screen's actual initial filter value ([#666](#666)) ([70a53d8](70a53d8)) * multi-farm tenancy decision record and AGENTS/GLOSSARY sync ([#537](#537)) ([#601](#601)) ([2c34771](2c34771)) * name the scoped filtered-empty key and state the [#653](#653) relationship plainly ([#666](#666)) ([0e93dac](0e93dac)) * note that a PackageReference in Directory.Build.props is invisible to the dependency graph ([4845724](4845724)) * **plans:** commit the [#722](#722) and [#745](#745) design records ([#754](#754)) ([c942fcd](c942fcd)) * record [#579](#579) as won't-fix — suspension is immediate for use, not issuance ([#582](#582)) ([7a3be40](7a3be40)) * record the [#508](#508) audit ordering key and the tracked-file guard lesson ([#701](#701)) ([08964e9](08964e9)) * **runbooks:** add procedure to rename the default farm's code after upgrade ([#731](#731)) ([2f6e242](2f6e242)) * screenshots of the running SPA in the README ([#550](#550)) ([711488a](711488a)) * **sim:** commit the dashboard screenshot, capture the palette matrix, and record the [#651](https://github.com/mforce/cluckwork/issues/651)/[#652](https://github.com/mforce/cluckwork/issues/652) conventions ([#660](#660), [#662](#662), [#663](#663), [#664](#664)) ([#665](#665)) ([930ea30](930ea30)) * specify searchable entity picker ([#641](#641)) ([91d4300](91d4300)) * split the README into audience-scoped docs and adopt repo-template scaffolding ([#548](#548)) ([b3f3fcf](b3f3fcf)) * surface Aspire local development workflow ([#568](#568)) ([a343baa](a343baa)) * **web:** record the per-screen idempotency-key policies and runWrite's refresh contract ([#703](#703)) ([#707](#707)) ([8bee651](8bee651)) * **web:** the date-cap help text covers every stocked item, not only feed ([#666](#666), [#667](#667)) ([c8433c5](c8433c5)) * **web:** the help text claims only what is true of recording, and says nothing about filter caps ([#666](#666), [#667](#667)) ([e2f63d1](e2f63d1)) * **web:** the help text describes the date-range filters that shipped ([#666](#666), [#667](#667)) ([c3275b7](c3275b7)) * **web:** the help text stops describing a cap the filters no longer have ([#666](#666), [#667](#667)) ([49654cd](49654cd)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: cluckwork-lockfix[bot] <309265648+cluckwork-lockfix[bot]@users.noreply.github.com>
1 participant
Closes #271 (slice S1 of epic #530).
Problem
Every API instance registered
DurableJobWorkeras a hosted service and polledPendingjobs with no row claim, and the three recurring sweeps (DailyEntryLockSweep,RefreshTokenPurgeSweep,IdempotencyRecordPurgeSweep) rode the same poll unconditionally. One instance is fine; two would each poll and both sweep.What this does
Gates the worker loop behind a Postgres session-scoped advisory lock (
pg_try_advisory_lock(271, 1)on a dedicated, non-pooled connection). The instance that acquires it is the leader and runs the poll + sweeps; a follower keeps its loop alive (health stays green) but does no work and re-attempts acquisition each poll. Crash recovery is automatic: a dead leader's session ends, Postgres releases the lock, and a survivor's next attempt wins.Deliberately a Postgres advisory lock, not one of #543's Redis ports (epic decision 13) — a Redis lock's per-replica fallback on an outage is exactly the double-execution the lease prevents.
Contract: at most one active leader, NOT "exactly once"
A session lock is mutual exclusion only. A leader can lose the lock mid-work (connection drop) and a process can crash after an external effect but before marking a job complete. So: durable jobs are at-least-once with idempotent handlers; recurring sweeps keep durable state; the lock only decides who may run. "Exactly once" appears nowhere in the code, tests, or this description.
Scope note — job-claim fencing deferred
There are no durable-job handlers yet (the poll logs "found N pending jobs; no handlers registered"), so the only live double-run is the sweeps, which the leader gate fully covers.
FOR UPDATE SKIP LOCKEDjob claiming is deferred to when handlers land — implementing it now would guard a code path nothing exercises.Health-check semantics
A follower stamps the heartbeat (it is a healthy, live worker doing its correct job of standing down), so
/healthdoes not read a follower as a stall. A faulted acquisition (could not reach the lock at all — e.g. a DB outage) is distinct: it backs off and leaves the heartbeat unstamped, so a sustained fault degrades/healthexactly as the pre-existing #69 stall-detection guarantee requires.Tests
PostgresLeaderLeaseTests(own Postgres container — advisory locks need no schema, and this keeps them isolated from the shared factory whose worker now holds the lock): two leases contend → exactly one leader; survivor reacquires after the leader releases (crash recovery); a leader whose session is killed relinquishes leadership and the freed lock is reacquirable (loss mid-work).DurableJobWorkerLeaderGateTests(no DB): leader polls; follower never polls but stays healthy; faulted lease never polls and does not stamp the heartbeat (the regression guard); a lease that wrongly throws is treated as a fault and the host survives.Every test was mutation-verified: each named test goes red when the specific guard it names is removed (leader gate, follower-vs-faulted routing, heartbeat stamp, session-liveness check, mutual exclusion, host-survives-on-throw).
Not in this PR
Transaction-pooling limitation (known, documented)
Single-leader is guaranteed on a session-pinned Postgres endpoint (a direct connection or a session-pooled proxy) — the reference/local topology. Under a transaction-pooling proxy (e.g. PgBouncer in transaction mode), the session advisory lock can be reassigned across backends: a backend-PID affinity check (captured in the same statement as the acquire, re-verified each poll) narrows the double-run window but does not close it, because the check runs at the start of each poll rather than on the acquire path. Under transaction pooling, safety therefore rests on the at-least-once + idempotent-sweep contract, not on single-leader. Making pooled deploys single-leader-safe (a dedicated session-pinned lease endpoint) is tracked as #556.
Review loop
Codex + local reviewers. Two product defects found and fixed: the Faulted-vs-Follower health regression, and the transaction-pooling overclaim (corrected to an honest limitation; real fix deferred to #556 per owner decision). AGENTS.md deploy-invariant sync is owned by #537 (T9) by design and flagged there. Loop stopped deliberately at this point — the remaining Codex P1 is the deferred #556 scope, not a defect in the shipped single-runner guarantee.