Skip to content

Let a re-authorization win over the repair sweeps - #2130

Merged
ppXD merged 1 commit into
mainfrom
fix/let-a-reauthorization-win-over-the-repair-sweep
Oct 19, 2026
Merged

ppXD merged 1 commit into
mainfrom
fix/let-a-reauthorization-win-over-the-repair-sweep

Conversation

@ppXD

@ppXD ppXD commented Oct 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • The stranded-binding repair, the connect-time carry-forward and the departed-member retirement each judge a credential dead and act on that verdict later. A re-authorization (or a provider accepting a refused token again) landing in between left bindings on a credential that was healthy again, or blanked a token the person had just been told was re-authorized.
  • Repair and retirement now try-acquire the credential's refresh lock (same semaphore and advisory key as the refresher and re-authorizer, so pods of either build exclude each other). A busy credential is skipped without waiting and met on a later pass; skipping is not a failure and never counts toward the sweep back-off. The dead verdict is re-read under the lock with the same query that picked the candidate, and the successor is the one that verdict names.
  • The refresh lock is taken by token writers only, and the hourly health probe revives a refused credential without it. The repair therefore also holds the dead credential's row (FOR NO KEY UPDATE, in a transaction of its own per credential) while it judges and moves, so a verdict recorded since the page was read is seen, and one that arrives later waits for the commit and finds the bindings already moved. The connect path holds the rows it read the same way for its unit of work and drops any whose status or payload changed.
  • No schema, API, lock-key or config change. The per-credential transaction is registered in ScopedTransactionInventoryTests as owned by design (a sweep has no transaction to join).

Test plan

  • Integration: interleaving tests with the gap made by a command interceptor on the sweep's own context. A re-authorization lands between the candidate read and the move, and bindings stay. A re-authorization arrives while the move is made, waits for the commit and lands after it. The health probe accepts a refused credential before the move, and bindings stay. The health probe arrives while the move is made, and waits for the commit. A busy credential (held by this process or by another pod) is skipped for more passes than the back-off tolerates and is then repaired or retired. A normal stranded credential is still repaired and the lock is given back.
  • Mutation checks, each turning tests red: drop the row hold, drop the under-lock re-check, drop the try-lock skip, make a skip count as a failure.
  • E2E (CredentialSweepLockE2ETests): a real HTTP reconnect queued behind a lock another pod holds, a repair pass in between, then the next pass; a real departed-member retirement against a held lock.
  • Regression: unit suite 14712 passed (1 pre-existing skip); credential/OAuth/sweep integration subset 420 plus 27 sweep-transaction tests passed; credential E2E subset 82 passed.
  • Staging: watch credential-binding-repair and departed-credential-retirement logs for "is being written ... stays a candidate" and for deadlock errors on the credential/repository rows after deploy.

@ppXD
ppXD changed the base branch from fix/let-an-owner-reconnect-their-own-credential to main October 19, 2026 03:05
The repair of stranded bindings, the connect-time carry-forward and the
departed-member retirement each judge a credential dead and act on that
verdict a moment later. An in-place re-authorization that lands in
between revives the credential, yet the move or the retirement went on:
the bindings left a credential that was healthy again, or its new token
was blanked and the person had been told the re-authorization worked.

Both sweeps now take the credential's refresh lock, the one every
rewrite of its token takes, without waiting. A credential somebody is
writing is skipped and met again on a later pass; otherwise the verdict
is taken again under the lock, by the same query that picked the
candidate, so a credential revived since the page was read moves
nothing. The successor is the one that verdict names.

The refresh lock is taken by the writers of the token only, and the
hourly health probe writes a refused credential back to Active, when its
provider accepts it again, without it. So the repair also holds the dead
credential's row (FOR NO KEY UPDATE, in a transaction of its own per
credential) and judges it under that hold: a verdict recorded since the
page was read is seen, and one that arrives later waits for the commit
and finds the bindings already moved.

A connect cannot hold the lock to the end of its unit of work without
leaking it on a rollback, so it holds the dead credential's row for the
transaction instead and drops any whose status or payload is no longer
what it read. A re-authorization that arrives after waits for the
commit and finds the bindings already moved.

Skipping a busy credential is not a failure: it never counts toward the
sweep's back-off, which the tests pin by skipping more passes than the
back-off tolerates before the lock is released.

No schema, API or lock-key change: the advisory key is the one the
refresher and the previous build already use, so pods of either build
exclude each other.
@ppXD
ppXD force-pushed the fix/let-a-reauthorization-win-over-the-repair-sweep branch from 90c3126 to 2dcd2be Compare October 19, 2026 03:05
@ppXD
ppXD merged commit 69e8b19 into main Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant