Skip to content

fix(auditdb): require matching anchor when refreshing an eid lease - #2120

Merged
AkramBitar merged 1 commit into
mainfrom
fix-2033-auditdb-locker-anchor
Aug 10, 2026
Merged

fix(auditdb): require matching anchor when refreshing an eid lease#2120
AkramBitar merged 1 commit into
mainfrom
fix-2033-auditdb-locker-anchor

Conversation

@AkramBitar

Copy link
Copy Markdown
Contributor

Fixes #2033

The problem

The lease row for an enrollment ID could be overwritten whenever owner = excluded.owner. owner is the replica ID — the same value for every audit on a node. So two concurrent audits on one node, for different transactions sharing an enrollment ID, both "acquired" it: the second overwrote the first's live row and AcquireLocks returned success to both. The mistake only surfaced later, at AssertLocksHeld just before the DB write.

The fix

Also require the anchor to match, so a lease is only refreshed by the same owner and the same transaction:

-- before
WHERE expires_at < NOW() OR owner = excluded.owner

-- after
WHERE expires_at < NOW() OR (owner = excluded.owner AND anchor = excluded.anchor)

Same owner, different anchor is now ordinary contention, retried until acquireDeadline. Expired leases stay claimable, so crash recovery is unchanged.

Tests

One query-shape test (no DB) and four against real Postgres: cross-anchor contention, same-anchor refresh, expired takeover, and four concurrent audits on one shared enrollment ID.

Against the old clause three fail — the concurrent one reporting all 4 audits as winners of the same lease. All pass with the fix under -race; go vet and golangci-lint clean.

@AkramBitar AkramBitar added this to the Q3/26 milestone Aug 3, 2026
@AkramBitar AkramBitar self-assigned this Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

📊 Token Validation Benchmark

Comparison of this PR against the base branch. 🟢 improvement · 🔴 regression · ➖ within ±1.0% noise.

Variant Benchmark Params Workers TPS (base → PR) Δ TPS
csp BenchmarkAPIGRPC f=1, nc=4, w=token-validation-service 4 151 → 150 ➖ -0.2%
csp BenchmarkLocalTokenValidation out-tokens=2in-tokens=2 4 120 → 120 ➖ +0.3%
ipa BenchmarkAPIGRPC f=1, nc=4, w=token-validation-service 4 91 → 91 ➖ -0.0%
ipa BenchmarkLocalTokenValidation out-tokens=2in-tokens=2 4 76 → 76 ➖ -0.1%

@AkramBitar
AkramBitar requested review from Effi-S and SaidAltury-ibm and removed request for HayimShaul August 5, 2026 14:01
@Effi-S

Effi-S commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

LGTM. @AkramBitar.
Notice you have a minor conflict

Signed-off-by: AkramBitar <akram@il.ibm.com>
@AkramBitar
AkramBitar force-pushed the fix-2033-auditdb-locker-anchor branch from a2b4e69 to 4697ba9 Compare August 10, 2026 16:01
@AkramBitar
AkramBitar merged commit 04845db into main Aug 10, 2026
222 of 227 checks passed
@Effi-S
Effi-S deleted the fix-2033-auditdb-locker-anchor branch August 13, 2026 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

auditdb: postgres locker's acquire upsert lets a node steal its own lock across unrelated anchors

2 participants