Context
Surfaced during adversarial review of PR LeanerCloud/cloud-commitments-cli#1227 (admin-bootstrap race fix). That PR introduced pg_advisory_xact_lock(8059058058580001) (a.k.a. adminInvariantAdvisoryLockKey) at the start of CreateAdminIfNone's tx in internal/auth/store_postgres.go, matching the same key the deferred trigger check_min_one_admin() already takes inside its body (migrations/000065_enforce_min_one_admin.up.sql:53).
This closes the bootstrap race (two concurrent first-admin creates) and is correct.
Asymmetry
Other admin-affecting writes (UpdateUser paths that flip active or drop the Administrators group, plus any future demote/delete paths) do not explicitly acquire the same advisory lock before doing their work. They rely entirely on the trg_min_one_admin_{delete,update} deferred constraint trigger to take the lock at commit time.
Functionally, the floor is still enforced — the deferred trigger acquires the lock just before its SELECT COUNT(*) so two simultaneous demote-the-last-admin commits still serialize through that one lock at commit time. But the lock acquisition is asymmetric:
- Bootstrap path:
Begin → pg_advisory_xact_lock(key) → INSERT → Commit
- Demote path:
Begin → UPDATE → Commit (trigger takes lock here)
A bootstrap caller blocked on the lock can be temporarily ordered after an update transaction that hasn't yet committed (and thus hasn't yet contended for the lock). The deferred trigger will still catch any final state where admin_count < 1, so we don't end up with zero admins. But the "lock is asymmetric" mental model is a footgun: the next person touching admin writes might assume holding the lock at the start of their tx is enough, only to be surprised that other writes don't.
Proposal
Add pg_advisory_xact_lock($adminInvariantAdvisoryLockKey) near the top of any UpdateUser / future DeactivateUser / demote path that might flip the active flag or group_ids for an existing Administrators member. Document the convention in internal/auth/store_postgres.go near the constant declaration.
Acceptance criteria
Priority
Low — current code is correct; this is a cleanup that reduces the chance of future mistakes.
Context
Surfaced during adversarial review of PR LeanerCloud/cloud-commitments-cli#1227 (admin-bootstrap race fix). That PR introduced
pg_advisory_xact_lock(8059058058580001)(a.k.a.adminInvariantAdvisoryLockKey) at the start ofCreateAdminIfNone's tx ininternal/auth/store_postgres.go, matching the same key the deferred triggercheck_min_one_admin()already takes inside its body (migrations/000065_enforce_min_one_admin.up.sql:53).This closes the bootstrap race (two concurrent first-admin creates) and is correct.
Asymmetry
Other admin-affecting writes (
UpdateUserpaths that flipactiveor drop the Administrators group, plus any future demote/delete paths) do not explicitly acquire the same advisory lock before doing their work. They rely entirely on thetrg_min_one_admin_{delete,update}deferred constraint trigger to take the lock at commit time.Functionally, the floor is still enforced — the deferred trigger acquires the lock just before its
SELECT COUNT(*)so two simultaneous demote-the-last-admin commits still serialize through that one lock at commit time. But the lock acquisition is asymmetric:Begin → pg_advisory_xact_lock(key) → INSERT → CommitBegin → UPDATE → Commit (trigger takes lock here)A bootstrap caller blocked on the lock can be temporarily ordered after an update transaction that hasn't yet committed (and thus hasn't yet contended for the lock). The deferred trigger will still catch any final state where
admin_count < 1, so we don't end up with zero admins. But the "lock is asymmetric" mental model is a footgun: the next person touching admin writes might assume holding the lock at the start of their tx is enough, only to be surprised that other writes don't.Proposal
Add
pg_advisory_xact_lock($adminInvariantAdvisoryLockKey)near the top of anyUpdateUser/ futureDeactivateUser/ demote path that might flip theactiveflag orgroup_idsfor an existing Administrators member. Document the convention ininternal/auth/store_postgres.gonear the constant declaration.Acceptance criteria
adminInvariantAdvisoryLockKeybefore issuing its UPDATE.adminInvariantAdvisoryLockKeystates the convention: "any tx that may demote/deactivate an Administrators-group member must acquire this lock".Priority
Low — current code is correct; this is a cleanup that reduces the chance of future mistakes.