diff --git a/docs/database-locks.md b/docs/database-locks.md index 04df4e6d..8cafb41b 100644 --- a/docs/database-locks.md +++ b/docs/database-locks.md @@ -26,7 +26,7 @@ advisory lock, `LOCK TABLE`, or an explicit `FOR SHARE`. | Lock | Key | Mode | Taken by | Serializes | |---|---|---|---|---| -| Materialization rendezvous | `hashtextextended('entitlement_materialization', 0)` | shared for a transaction that materializes a pool; exclusive for a rule change | the `entitlements.Materializing` and `entitlements.RuleChange` transaction kinds (`BeginMaterializing`, `BeginRuleChange`), as the transaction's first statement | A rule change against every materializing transaction: each one finishes before the change or starts after it. Materializing transactions do not wait on each other here. | +| Materialization rendezvous | `hashtextextended('entitlement_materialization', 0)` | shared for a transaction that materializes a pool; exclusive for a rule change and for a change to a ladder's tiers (tier add, tier delete, tier removal, the reorder's renumbering) | the `entitlements.Materializing` and `entitlements.RuleChange` transaction kinds (`BeginMaterializing`, `BeginRuleChange`), as the transaction's first statement | A rule change or a tier change against every materializing transaction: each one finishes before the change or starts after it, so a conferral reads one set of rules and one order of each ladder. Materializing transactions do not wait on each other here. | | Subscription | `hashtext()::bigint` | exclusive | `fulfillment.ReconcileSubscription` | Two reconciles of one subscription (the checkout return and a webhook), so they converge instead of racing the exclusion constraint on subscription periods. | | Invoice | `hashtext()::bigint` | exclusive | `invoiceReconcile.converge` in `internal/integrations/stripe/workflows` | Two reconciles of one invoice (two events, or an event and the sweep), so the second finds what the first wrote. | | Product sync | `hashtext()::bigint` | exclusive | `SyncProductToStripe` in `internal/server/operator_billing.go` | Two sync clicks for one product, so the mapping writes and the outbox enqueue happen once. | @@ -150,23 +150,29 @@ advisory lock, then the rest by branch. ### Tier add, tier removal, default change and reorder -All of these run in transactions from `BeginMaterializing`. +The tier add, the tier removal, the tier delete and the reorder's +renumbering run in transactions from `BeginRuleChange`, which takes the +exclusive rendezvous: each changes a ladder's tiers, and `core.confer` reads a +ladder's ranks and a product's shape in separate statements, so no conferral +may run while one of them commits (#213). The default change and each +organization's enactment run in transactions from `BeginMaterializing`. -- **Tier add** (`CreatePlanLadderTier`): the rendezvous, the tier insert, then - for each live provision of the product in creation order +- **Tier add** (`CreatePlanLadderTier`): the exclusive rendezvous, the tier + insert, then for each live provision of the product in creation order `core.align_conferral_shape` (the pool row of that provision) and materialization. The pools are locked in provision order, not in `pool_id` order. - **Tier removal with holders** (`CommitPlanLadderTierRemoval`): the - rendezvous, then the pool row of every holder through `LockPools`, which sorts - and deduplicates them, so they lock in ascending `pool_id` order, then the - holders are read again under the locks, the tier + exclusive rendezvous, then the pool row of every holder through + `LockPools`, which sorts and deduplicates them, so they lock in ascending + `pool_id` order, then the holders are read again under the locks, the tier is deleted and the ranks renumbered. Then, for each pool in the same order, the incumbents end (`core.end_conferral`), the remaining positions align (`core.align_conferral_shape`), the floor is restored when the disposition is migrate, and the pool materializes. -- **Tier delete without holders** (`DeletePlanLadderTier`): the rendezvous - only, with no pool lock, around the tier delete and the renumbering. +- **Tier delete without holders** (`DeletePlanLadderTier`): the exclusive + rendezvous only, with no pool lock, around the tier delete and the + renumbering. - **Default change** (`CommitOrgTypeDefaultChange` and `enactOrgDisposition`): the default is saved outside any lock beyond the row update. Then each organization is its own transaction: the rendezvous, the organization's @@ -174,10 +180,10 @@ All of these run in transactions from `BeginMaterializing`. (`ReapplyDefaultsIfVacant`, or `ConferGrantTx` for a grandfathered incumbent, or `core.end_conferral` and materialization for a migration). A failure on one organization leaves the others alone. -- **Reorder** (`ReorderPlanLadderTiers`): the rendezvous around the +- **Reorder** (`ReorderPlanLadderTiers`): the exclusive rendezvous around the renumbering, committed with no pool lock. When the rank-0 tier changed, each - affected organization then runs `enactOrgDisposition`, as a default change - does. + affected organization then runs `enactOrgDisposition` under the shared + rendezvous, as a default change does. ### Invoice reconcile @@ -276,8 +282,12 @@ that allocates or releases the name. ## Rules every path keeps 1. A transaction that materializes a pool takes the shared rendezvous as its - first statement, through `BeginMaterializing`. A rule change takes the - exclusive one through `BeginRuleChange`. `core.assert_rendezvous` raises + first statement, through `BeginMaterializing`. A rule change or an + operator's tier change (the tier add, the tier delete, the tier removal, + the reorder's renumbering) takes the exclusive one through + `BeginRuleChange`, and may materialize under it; the demonstration seed + adds its ladder's tiers on the plain handle, outside the rendezvous. + `core.assert_rendezvous` raises `materialization_rendezvous_missing` when a function that materializes or settles runs without it. 2. The pool row is locked before positions or entitlements change, except in diff --git a/docs/go-conventions.md b/docs/go-conventions.md index fe20f28c..8ffeaafd 100644 --- a/docs/go-conventions.md +++ b/docs/go-conventions.md @@ -65,12 +65,14 @@ a kind. `db.InTx(ctx, database, kind, fn)` commits when `fn` returns nil and rolls back on an error or a panic. `db.Begin` returns the transaction for a caller that needs to keep it open across several steps. A kind says how the transaction opens: `db.Plain` and `db.ReadCommitted` open with nothing run -first, and a transaction that materializes a pool or changes a rule opens with -`entitlements.Materializing` or `entitlements.RuleChange`, which set -`lock_timeout` and take the materialization rendezvous before anything else -(`entitlements.BeginMaterializing` and `BeginRuleChange` are those two calls). -A failed begin, a failed commit and `fn`'s own error come back unwrapped; the -caller adds its own context once around the whole act. +first. A transaction that materializes a pool opens with +`entitlements.Materializing`, and one that changes a rule or carries an +operator's tier change with `entitlements.RuleChange`, whose rendezvous is +exclusive; both set `lock_timeout` and take the materialization rendezvous +before anything else (`entitlements.BeginMaterializing` and `BeginRuleChange` +are those two calls). A failed begin, a failed commit and `fn`'s own error +come back unwrapped; the caller adds its own context once around the whole +act. A lock on a pool row goes through the `entitlements.Queries` methods `LockPools` (any number of pools; it sorts them, takes each once, and returns diff --git a/internal/entitlements/tx.go b/internal/entitlements/tx.go index 8fb05364..dbef0908 100644 --- a/internal/entitlements/tx.go +++ b/internal/entitlements/tx.go @@ -17,9 +17,11 @@ import ( // The materialization rendezvous (Decision 144): one transaction-scoped // advisory lock that every transaction materializing a pool holds in shared -// mode and every transaction changing a rule holds in exclusive mode, taken -// as the transaction's first statement so no holder ever waits on something -// a would-be holder already holds. The key is hashtextextended of this name. +// mode and every rule change and operator tier change holds in exclusive +// mode, taken as the transaction's first statement so no holder ever waits +// on something a would-be holder already holds. The demonstration seed adds +// its ladder's tiers on the plain handle, outside it. The key is +// hashtextextended of this name. const rendezvousName = "entitlement_materialization" // rendezvousLockTimeout bounds every wait on the rendezvous; a wait past it @@ -39,10 +41,14 @@ const rendezvousMissingMessage = "materialization_rendezvous_missing" // queued behind a rule commit would read the rules as they were before it. var Materializing = db.FirstRuns(rendezvous("pg_advisory_xact_lock_shared")) -// RuleChange opens a transaction that changes a rule: it sets the lock timeout -// and takes the exclusive rendezvous before anything else, so every -// materializing transaction either finished before the change or starts -// after it. +// RuleChange opens a transaction that changes a rule or carries an +// operator's tier change (a tier added, deleted or removed, or the ranks +// renumbered): it sets the lock timeout and takes the exclusive rendezvous +// before anything else, so every materializing transaction either finished +// before the change or starts after it. core.confer reads a ladder's ranks +// and a product's shape in separate statements, so a tier change that +// committed between them would have it classify a tier change against two +// orders of the ladder (#213). var RuleChange = db.FirstRuns(rendezvous("pg_advisory_xact_lock")) // BeginMaterializing opens a Materializing transaction on b. diff --git a/internal/server/operator_plan_ladders.go b/internal/server/operator_plan_ladders.go index 4a4fc3a6..a1518638 100644 --- a/internal/server/operator_plan_ladders.go +++ b/internal/server/operator_plan_ladders.go @@ -406,7 +406,7 @@ func (h *OperatorPartialsHandler) CreatePlanLadderTier(w http.ResponseWriter, r // product run in one tx: adding the tier grew the product's conferral shape, // so existing deliveries must gain the new rung atomically. ctx := r.Context() - tx, err := entitlements.BeginMaterializing(ctx, h.Database) + tx, err := entitlements.BeginRuleChange(ctx, h.Database) if err != nil { h.Logger.Error("failed to begin add-tier tx", slog.Any("error", err)) h.renderPlanLadderTiersPage(w, r, ladderID, "", "Failed to add tier; see server logs.") @@ -695,7 +695,7 @@ func (h *OperatorPartialsHandler) ReorderPlanLadderTiers(w http.ResponseWriter, return } - tx, err := entitlements.BeginMaterializing(ctx, h.Database) + tx, err := entitlements.BeginRuleChange(ctx, h.Database) if err != nil { h.Logger.Error("failed to begin tier reorder tx", slog.Any("error", err)) h.renderPlanLadderTiersPage(w, r, ladderID, "", "Failed to reorder tiers.") @@ -846,7 +846,7 @@ func (h *OperatorPartialsHandler) DeletePlanLadderTier(w http.ResponseWriter, r // ranks never gap and the ladder always has a rank-0 provisioning default // (deleting the base tier promotes the next tier — the confirm dialog says so). ctx := r.Context() - tx, err := entitlements.BeginMaterializing(ctx, h.Database) + tx, err := entitlements.BeginRuleChange(ctx, h.Database) if err != nil { h.Logger.Error("failed to begin tier delete tx", slog.Any("error", err)) h.renderPlanLadderTiersPage(w, r, ladderID, "", "Failed to remove tier; see server logs.") @@ -1075,7 +1075,7 @@ func (h *OperatorPartialsHandler) CommitPlanLadderTierRemoval(w http.ResponseWri Reason: fmt.Sprintf("tier removal (%s from %s)", productName, ladderLabel), } - tx, err := entitlements.BeginMaterializing(ctx, h.Database) + tx, err := entitlements.BeginRuleChange(ctx, h.Database) if err != nil { h.Logger.Error("failed to begin tier removal tx", slog.Any("error", err)) h.renderPlanLadderTiersPage(w, r, ladderID, "", tierRemovalRefusal(err)) diff --git a/internal/server/operator_tier_reorder_test.go b/internal/server/operator_tier_reorder_test.go index 9b0ec6c0..6ab2d95d 100644 --- a/internal/server/operator_tier_reorder_test.go +++ b/internal/server/operator_tier_reorder_test.go @@ -381,9 +381,12 @@ func TestAddTierInductionAlignsLiveProvisions(t *testing.T) { } // A tier-reorder commit racing an org-type default-change commit on the same -// pool serializes on the per-pool FOR UPDATE lock: whichever enacts first -// grandfathers the incumbent, the other re-derives the bucket in-tx and finds -// a legacy-held (other-source) pool — never a second grandfather. +// pool. The two enactments serialize on the per-pool FOR UPDATE lock: +// whichever enacts first grandfathers the incumbent, the other re-derives the +// bucket in-tx and finds a legacy-held (other-source) pool — never a second +// grandfather. The reorder's renumbering takes the rendezvous exclusively, so +// it commits before or after each enactment's conferral, never between that +// conferral's rank reads, and the grandfather is recorded as one transfer. func TestTierReorder_CommitRacesDefaultChangeCommit(t *testing.T) { database := dbtest.Open(t) f := newOtcFixture(t, database) diff --git a/internal/server/plan_acts_concurrency_db_test.go b/internal/server/plan_acts_concurrency_db_test.go index a70f2b8c..42033570 100644 --- a/internal/server/plan_acts_concurrency_db_test.go +++ b/internal/server/plan_acts_concurrency_db_test.go @@ -10,9 +10,9 @@ package server // No test sleeps to choose an order. What came of each interleaving is // compared with a file under testdata/plan_acts, written as the acts behave // today: the save-first default change, a removal that holds every holder's -// pool, a renumbering that takes the rendezvous in shared mode only, so a -// conferral on the ladder running at the same moment reads the ranks of two -// orders, and a reorder that enacts against the rank 0 it saved. +// pool, a change to a ladder's tiers that takes the rendezvous exclusively, so +// it waits for every conferral in flight, and a reorder that enacts against +// the rank 0 it saved. import ( "context" @@ -25,6 +25,7 @@ import ( "github.com/lib/pq" + "git.coopcloud.tech/wiki-cafe/member-console/internal/billing" "git.coopcloud.tech/wiki-cafe/member-console/internal/db" "git.coopcloud.tech/wiki-cafe/member-console/internal/entitlements" "git.coopcloud.tech/wiki-cafe/member-console/internal/testkit/authtest" @@ -33,6 +34,13 @@ import ( "git.coopcloud.tech/wiki-cafe/member-console/internal/testkit/world" ) +// The rendezvous statements as pg_stat_activity shows them, for a wait on one +// mode. The exclusive text is not part of the shared one. +const ( + exclusiveRendezvous = "pg_advisory_xact_lock(hashtextextended" + sharedRendezvous = "pg_advisory_xact_lock_shared(hashtextextended" +) + // sideTx is a transaction on its own connection that holds row locks a test // releases when it chooses. It takes no rendezvous. type sideTx struct { @@ -81,6 +89,16 @@ func (s *sideTx) lockProvision(provisionID string) *sideTx { return s } +// lockTier holds a ladder's tier row, so a renumbering of the ladder waits +// when it reaches the tier, still holding what it took before it. +func (s *sideTx) lockTier(l world.Ladder, p world.Product) *sideTx { + s.t.Helper() + if _, err := s.tx.ExecContext(context.Background(), `SELECT 1 FROM core.plan_ladder_tiers WHERE plan_ladder_id = $1 AND product_id = $2 FOR UPDATE`, l.ID, p.ID); err != nil { + s.t.Fatalf("lock tier %s of %s: %v", p.Name, l.Name, err) + } + return s +} + // pid is the side transaction's backend, for a wait on the locks it holds. func (s *sideTx) pid() int { s.t.Helper() @@ -178,9 +196,11 @@ func removalAgainstDefault(f *actFixture) (holder world.Org, change, removal *ac } // TestTierRemovalAgainstDefaultChange runs a tier removal and a default -// change on one pool in each order, queued on the pool's row and one after -// the other. Every case leaves T defaulting to Next and no live position on -// a tier the removal deleted. +// change on one pool in each order, queued and one after the other. Queued, +// the first waits on the pool's row and the second on the rendezvous, which +// the removal takes exclusively and the default change's enactment shares. +// Every case leaves T defaulting to Next and no live position on a tier the +// removal deleted. func TestTierRemovalAgainstDefaultChange(t *testing.T) { cases := []struct { name string @@ -192,7 +212,7 @@ func TestTierRemovalAgainstDefaultChange(t *testing.T) { first := change.start() f.awaitQueued("LockResourcePool", 1) second := removal.start() - f.awaitQueued("LockResourcePool", 2) + f.awaitQueued(exclusiveRendezvous, 1) side.release() return pair{First: first.wait(), Second: second.wait()} }}, @@ -202,7 +222,7 @@ func TestTierRemovalAgainstDefaultChange(t *testing.T) { first := removal.start() f.awaitQueued("LockResourcePool", 1) second := change.start() - f.awaitQueued("LockResourcePool", 2) + f.awaitQueued(sharedRendezvous, 1) side.release() return pair{First: first.wait(), Second: second.wait()} }}, @@ -329,14 +349,15 @@ func renumberingDuringConferral(f *actFixture) (main world.Ladder, plus world.Pr return main, plus, holder, change } -// TestRenumberingCommitsDuringAConferral pauses a conferral on a ladder -// between its rank reads (a side transaction holds the provision it -// supersedes) and commits a renumbering of that ladder meanwhile. Each of -// the three renumberings takes the rendezvous in shared mode and no lock the -// conferral holds, so it commits, and the conferral records its transition -// with the rank it read before and the rank it reads after. The files pin -// that transition as it is recorded today, not the transfer that belongs. -func TestRenumberingCommitsDuringAConferral(t *testing.T) { +// TestTierChangesWaitForAConferral pauses a conferral on a ladder between its +// rank reads (a side transaction holds the provision it supersedes) and sends +// a change to that ladder's tiers meanwhile: a reorder, a tier delete and a +// tier removal renumber it, and a tier add grows the shape of the product the +// conferral confers. Each takes the rendezvous exclusively, so it queues +// behind the conferral's transaction and commits after it, and the conferral +// reads one order of the ladder: Holder's grandfather is a transfer at one +// rank (#213). +func TestTierChangesWaitForAConferral(t *testing.T) { cases := []struct { name string run func(f *actFixture) pair @@ -360,21 +381,33 @@ func TestRenumberingCommitsDuringAConferral(t *testing.T) { f.w.Issue(f.org("Granted", f.orgType("U", nil)), plus, world.IssueOpts{By: f.operator}) return f.duringConferral(holder, change, f.prepare(actRemoveTier, actDisposition("keep"), actOnTier(main, plus)...)) }}, + {"a tier add", func(f *actFixture) pair { + basic, gold := f.product("Basic", 1), f.product("Gold", 5) + main := f.ladder("Main", basic) + next := f.ladder("Next", gold) + other := f.ladder("Other") + ot := f.orgType("T", &main) + holder := f.floored("Holder", ot) + change := f.prepare(actChangeDefault, actNewDefault(&next, "grandfather"), authtest.PathValue("orgType", ot)) + add := f.prepare(actAddTier, map[string][]string{"product_id": {basic.ID}}, authtest.PathValue("ladderID", other.ID)) + return f.duringConferral(holder, change, add) + }}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { f := newActFixture(t) got := c.run(f) got.Rows = f.rows() - f.compareJSON(goldenCase("renumbering-during-conferral", c.name), got) + f.compareJSON(goldenCase("tier-change-during-conferral", c.name), got) }) } } // duringConferral holds Holder's default provision, starts the default -// change, waits until its conferral waits on the provision, sends the -// renumbering to completion, and releases the provision. -func (f *actFixture) duringConferral(holder world.Org, change, renumbering *actCall) pair { +// change, waits until its conferral waits on the provision, starts the tier +// change and waits until it queues on the rendezvous, then releases the +// provision. +func (f *actFixture) duringConferral(holder world.Org, change, tierChange *actCall) pair { f.t.Helper() var provisionID string if err := f.w.DB.QueryRowContext(f.w.Ctx, `SELECT provision_id::text FROM core.pool_provisions @@ -384,10 +417,11 @@ func (f *actFixture) duringConferral(holder world.Org, change, renumbering *actC side := f.side().lockProvision(provisionID) conferring := change.start() dbtest.AwaitLockWait(f.t, f.w.DB, dbtest.Backend{BlockedBy: side.pid()}) - second := renumbering.reply(renumbering.send()) + changing := tierChange.start() + f.awaitQueued(exclusiveRendezvous, 1) side.release() first := conferring.wait() - return pair{First: first, Second: second} + return pair{First: first, Second: changing.wait()} } // goldenCase is the file of one case of a test: testdata/plan_acts// @@ -396,29 +430,63 @@ func goldenCase(test, name string) string { return filepath.Join("testdata", "plan_acts", test, strings.ReplaceAll(name, " ", "-")+".json") } -// TestReorderEnactsAgainstTheRankZeroItSaved sends two reorders of one -// ladder whose enactments both queue on Holder's pool. The first saved Plus -// as rank 0 and enacts against it; by the time it holds the pool the second -// has put Basic back at rank 0, so the first grandfathers Holder onto a -// legacy grant of Basic, the rank 0 of the ladder as it now stands, and the -// second finds Holder on that grant and leaves it (#187). +// TestReorderEnactsAgainstTheRankZeroItSaved commits a renumbering of the +// ladder between a reorder's own renumbering and its enactment. The reorder +// saved Plus as rank 0 and enacts against it; by then the ladder has Basic at +// rank 0 again, so it grandfathers Holder onto a legacy grant of Basic, the +// rank 0 of the ladder as it now stands (#187). A side transaction holds +// Basic's tier row, so the reorder waits inside its renumbering while it +// holds the exclusive rendezvous; the other renumbering queues on the +// rendezvous behind it, and PostgreSQL grants it the lock when the reorder +// commits, before the reorder's enactment asks for the lock. func TestReorderEnactsAgainstTheRankZeroItSaved(t *testing.T) { f := newActFixture(t) basic, plus := f.product("Basic", 1), f.product("Plus", 2) main := f.ladder("Main", basic, plus) - holder := f.floored("Holder", f.orgType("T", &main)) - path := authtest.PathValue("ladderID", main.ID) - first := f.prepare(actReorderTiers, actOrder("grandfather", plus, basic), path) - second := f.prepare(actReorderTiers, actOrder("grandfather", basic, plus), path) + f.floored("Holder", f.orgType("T", &main)) + reorder := f.prepare(actReorderTiers, actOrder("grandfather", plus, basic), authtest.PathValue("ladderID", main.ID)) - side := f.side().lockPool(holder.PoolID) - one := first.start() - f.awaitQueued("LockResourcePool", 1) - two := second.start() - f.awaitQueued("LockResourcePool", 2) + side := f.side().lockTier(main, basic) + one := reorder.start() + dbtest.AwaitLockWait(t, f.w.DB, dbtest.Backend{BlockedBy: side.pid()}) + renumbered := f.renumberQueued(main, basic, plus) + f.awaitQueued(exclusiveRendezvous, 1) side.release() - got := pair{First: one.wait(), Second: two.wait()} + got := struct { + First actReply `json:"first"` + Rows actRows `json:"rows"` + }{First: one.wait()} + if err := <-renumbered; err != nil { + t.Fatalf("renumber Main: %v", err) + } got.Rows = f.rows() f.compareJSON(filepath.Join("testdata", "plan_acts", "stale-candidate.json"), got) } + +// renumberQueued renumbers l's tiers in the order given, as another reorder's +// renumbering does, in a transaction of its own on its own goroutine. It takes +// the exclusive rendezvous first, so it queues behind any transaction that +// holds the rendezvous. The channel gives its error once it has committed. +func (f *actFixture) renumberQueued(l world.Ladder, order ...world.Product) <-chan error { + ids := make([]string, len(order)) + for i, p := range order { + ids[i] = p.ID + } + done := make(chan error, 1) + go func() { + ctx := context.Background() + tx, err := entitlements.BeginRuleChange(ctx, f.w.DB) + if err != nil { + done <- err + return + } + defer func() { _ = tx.Rollback() }() + if err := renumberTiers(ctx, billing.New(tx), l.ID, ids); err != nil { + done <- err + return + } + done <- tx.Commit() + }() + return done +} diff --git a/internal/server/testdata/plan_acts/add-aligns-a-live-provision.json b/internal/server/testdata/plan_acts/add-aligns-a-live-provision.json index cce61d78..14646670 100644 --- a/internal/server/testdata/plan_acts/add-aligns-a-live-provision.json +++ b/internal/server/testdata/plan_acts/add-aligns-a-live-provision.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "CreatePlanLadderTier", "SELECT plan_ladder_id, action FROM core.align_conferral_shape($1, $2, $3, $4, $5)", "SELECT core.assert_rendezvous(false)", diff --git a/internal/server/testdata/plan_acts/add-collides-with-a-held-rung.json b/internal/server/testdata/plan_acts/add-collides-with-a-held-rung.json index 583fba95..ea55187c 100644 --- a/internal/server/testdata/plan_acts/add-collides-with-a-held-rung.json +++ b/internal/server/testdata/plan_acts/add-collides-with-a-held-rung.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "CreatePlanLadderTier", "SELECT plan_ladder_id, action FROM core.align_conferral_shape($1, $2, $3, $4, $5)", "rollback" @@ -19,7 +19,7 @@ "GetProductByID", "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "CreatePlanLadderTier", "GetLivePoolProvisionsByProductID", "SELECT plan_ladder_id, action FROM core.align_conferral_shape($1, $2, $3, $4, $5)", diff --git a/internal/server/testdata/plan_acts/delete-bottom-tier.json b/internal/server/testdata/plan_acts/delete-bottom-tier.json index da3c8347..47947689 100644 --- a/internal/server/testdata/plan_acts/delete-bottom-tier.json +++ b/internal/server/testdata/plan_acts/delete-bottom-tier.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "DeleteTier", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/delete-top-tier.json b/internal/server/testdata/plan_acts/delete-top-tier.json index 3d524feb..c9606dae 100644 --- a/internal/server/testdata/plan_acts/delete-top-tier.json +++ b/internal/server/testdata/plan_acts/delete-top-tier.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "DeleteTier", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/remove-keep-rank-0.json b/internal/server/testdata/plan_acts/remove-keep-rank-0.json index 8991a498..2a516b76 100644 --- a/internal/server/testdata/plan_acts/remove-keep-rank-0.json +++ b/internal/server/testdata/plan_acts/remove-keep-rank-0.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "LockResourcePool", "DeleteTier", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/remove-last-tier-keep.json b/internal/server/testdata/plan_acts/remove-last-tier-keep.json index 1c492df4..03db29d8 100644 --- a/internal/server/testdata/plan_acts/remove-last-tier-keep.json +++ b/internal/server/testdata/plan_acts/remove-last-tier-keep.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "LockResourcePool", "DeleteTier", "SELECT plan_ladder_id, action FROM core.align_conferral_shape($1, $2, $3, $4, $5)", diff --git a/internal/server/testdata/plan_acts/remove-last-tier-migrate.json b/internal/server/testdata/plan_acts/remove-last-tier-migrate.json index cb9776fd..2de0b2dc 100644 --- a/internal/server/testdata/plan_acts/remove-last-tier-migrate.json +++ b/internal/server/testdata/plan_acts/remove-last-tier-migrate.json @@ -11,7 +11,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "LockResourcePool", "DeleteTier", "SELECT core.end_conferral($1, $2, $3, $4, $5, $6, $7, $8)", diff --git a/internal/server/testdata/plan_acts/remove-migrate-pool-held-elsewhere.json b/internal/server/testdata/plan_acts/remove-migrate-pool-held-elsewhere.json index 8b4d11e4..100e228c 100644 --- a/internal/server/testdata/plan_acts/remove-migrate-pool-held-elsewhere.json +++ b/internal/server/testdata/plan_acts/remove-migrate-pool-held-elsewhere.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "LockResourcePool", "DeleteTier", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/remove-migrate-rank-0.json b/internal/server/testdata/plan_acts/remove-migrate-rank-0.json index 8d36c6c8..d6f7aac2 100644 --- a/internal/server/testdata/plan_acts/remove-migrate-rank-0.json +++ b/internal/server/testdata/plan_acts/remove-migrate-rank-0.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "LockResourcePool", "DeleteTier", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/remove-needs-disposition.json b/internal/server/testdata/plan_acts/remove-needs-disposition.json index 64692a62..13bee32d 100644 --- a/internal/server/testdata/plan_acts/remove-needs-disposition.json +++ b/internal/server/testdata/plan_acts/remove-needs-disposition.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "LockResourcePool", "rollback" ], diff --git a/internal/server/testdata/plan_acts/remove-no-holders-left.json b/internal/server/testdata/plan_acts/remove-no-holders-left.json index ae6bf8d0..ca4a9caa 100644 --- a/internal/server/testdata/plan_acts/remove-no-holders-left.json +++ b/internal/server/testdata/plan_acts/remove-no-holders-left.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "DeleteTier", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/remove-other-source.json b/internal/server/testdata/plan_acts/remove-other-source.json index 85e91b8a..0fec6bc7 100644 --- a/internal/server/testdata/plan_acts/remove-other-source.json +++ b/internal/server/testdata/plan_acts/remove-other-source.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "LockResourcePool", "DeleteTier", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/remove-product-name-unread.json b/internal/server/testdata/plan_acts/remove-product-name-unread.json index 0454af59..34137b12 100644 --- a/internal/server/testdata/plan_acts/remove-product-name-unread.json +++ b/internal/server/testdata/plan_acts/remove-product-name-unread.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "LockResourcePool", "DeleteTier", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/remove-second-pool-fails.json b/internal/server/testdata/plan_acts/remove-second-pool-fails.json index 174695c7..ee7cdac4 100644 --- a/internal/server/testdata/plan_acts/remove-second-pool-fails.json +++ b/internal/server/testdata/plan_acts/remove-second-pool-fails.json @@ -11,7 +11,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "LockResourcePool", "LockResourcePool", "DeleteTier", diff --git a/internal/server/testdata/plan_acts/remove-two-pools.json b/internal/server/testdata/plan_acts/remove-two-pools.json index 2c9fc2b2..0aa8f2eb 100644 --- a/internal/server/testdata/plan_acts/remove-two-pools.json +++ b/internal/server/testdata/plan_acts/remove-two-pools.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "LockResourcePool", "LockResourcePool", "DeleteTier", diff --git a/internal/server/testdata/plan_acts/reorder-an-organization-fails.json b/internal/server/testdata/plan_acts/reorder-an-organization-fails.json index 772ed00d..9bdecd7b 100644 --- a/internal/server/testdata/plan_acts/reorder-an-organization-fails.json +++ b/internal/server/testdata/plan_acts/reorder-an-organization-fails.json @@ -11,7 +11,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "UpdateTierRank", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/reorder-cosmetic.json b/internal/server/testdata/plan_acts/reorder-cosmetic.json index eea8b9b1..97bf8d22 100644 --- a/internal/server/testdata/plan_acts/reorder-cosmetic.json +++ b/internal/server/testdata/plan_acts/reorder-cosmetic.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "UpdateTierRank", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/reorder-grandfather.json b/internal/server/testdata/plan_acts/reorder-grandfather.json index 95dff768..0a223460 100644 --- a/internal/server/testdata/plan_acts/reorder-grandfather.json +++ b/internal/server/testdata/plan_acts/reorder-grandfather.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "UpdateTierRank", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/reorder-list-fails.json b/internal/server/testdata/plan_acts/reorder-list-fails.json index 4e3361ed..b2543506 100644 --- a/internal/server/testdata/plan_acts/reorder-list-fails.json +++ b/internal/server/testdata/plan_acts/reorder-list-fails.json @@ -11,7 +11,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "UpdateTierRank", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/reorder-migrate.json b/internal/server/testdata/plan_acts/reorder-migrate.json index 31159f75..4d6d5e19 100644 --- a/internal/server/testdata/plan_acts/reorder-migrate.json +++ b/internal/server/testdata/plan_acts/reorder-migrate.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "UpdateTierRank", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/reorder-org-types-unread.json b/internal/server/testdata/plan_acts/reorder-org-types-unread.json index 859c5231..715181f6 100644 --- a/internal/server/testdata/plan_acts/reorder-org-types-unread.json +++ b/internal/server/testdata/plan_acts/reorder-org-types-unread.json @@ -11,7 +11,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "UpdateTierRank", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/reorder-rank-0-no-default.json b/internal/server/testdata/plan_acts/reorder-rank-0-no-default.json index e1294659..a2cb3694 100644 --- a/internal/server/testdata/plan_acts/reorder-rank-0-no-default.json +++ b/internal/server/testdata/plan_acts/reorder-rank-0-no-default.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "UpdateTierRank", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/reorder-redrop.json b/internal/server/testdata/plan_acts/reorder-redrop.json index 63e56a76..6901f7b6 100644 --- a/internal/server/testdata/plan_acts/reorder-redrop.json +++ b/internal/server/testdata/plan_acts/reorder-redrop.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "UpdateTierRank", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/reorder-stale.json b/internal/server/testdata/plan_acts/reorder-stale.json index 4501b54a..bd72d35b 100644 --- a/internal/server/testdata/plan_acts/reorder-stale.json +++ b/internal/server/testdata/plan_acts/reorder-stale.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "UpdateTierRank", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/reorder-two-types.json b/internal/server/testdata/plan_acts/reorder-two-types.json index 380fcd90..90a0d7e6 100644 --- a/internal/server/testdata/plan_acts/reorder-two-types.json +++ b/internal/server/testdata/plan_acts/reorder-two-types.json @@ -8,7 +8,7 @@ "statements": [ "begin", "SET LOCAL lock_timeout = '5s'", - "SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", "UpdateTierRank", "UpdateTierRank", "UpdateTierRank", diff --git a/internal/server/testdata/plan_acts/stale-candidate.json b/internal/server/testdata/plan_acts/stale-candidate.json index e21f3a91..46e65513 100644 --- a/internal/server/testdata/plan_acts/stale-candidate.json +++ b/internal/server/testdata/plan_acts/stale-candidate.json @@ -7,14 +7,6 @@ "checked": [], "logs": [] }, - "second": { - "answer": [ - "status 200", - "Hx-Trigger showSuccessToast: Basic is now the default plan for new T organizations: 1 untouched." - ], - "checked": [], - "logs": [] - }, "rows": { "tiers": [ "ladder | rank | product", diff --git a/internal/server/testdata/plan_acts/renumbering-during-conferral/a-reorder.json b/internal/server/testdata/plan_acts/tier-change-during-conferral/a-reorder.json similarity index 92% rename from internal/server/testdata/plan_acts/renumbering-during-conferral/a-reorder.json rename to internal/server/testdata/plan_acts/tier-change-during-conferral/a-reorder.json index c0f00030..a2400448 100644 --- a/internal/server/testdata/plan_acts/renumbering-during-conferral/a-reorder.json +++ b/internal/server/testdata/plan_acts/tier-change-during-conferral/a-reorder.json @@ -45,7 +45,7 @@ "pool | provision | ladder | from_rank | to_rank | transition_type | actor_type | actor | reason", " | | | - | 0 | initiate | system | none | test fixture", " | | | 0 | - | end | operator | | org-type default change (): grandfathered as legacy", - " | | | 0 | 1 | upgrade | operator | | org-type default change (): grandfathered as legacy" + " | | | 0 | 0 | transfer | operator | | org-type default change (): grandfathered as legacy" ], "entitlements": [ "pool | resource_key | resource_limit", diff --git a/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-add.json b/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-add.json new file mode 100644 index 00000000..90d69e51 --- /dev/null +++ b/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-add.json @@ -0,0 +1,57 @@ +{ + "first": { + "answer": [ + "status 200", + "Hx-Trigger showSuccessToast: Default change committed for : 1 grandfathered." + ], + "checked": [], + "logs": [] + }, + "second": { + "answer": [ + "status 200", + "Hx-Trigger showSuccessToast: Tier added at the bottom; drag it into position." + ], + "checked": [], + "logs": [] + }, + "rows": { + "tiers": [ + "ladder | rank | product", + " | 0 | ", + " | 0 | ", + " | 0 | " + ], + "defaults": [ + "org_type | default_ladder", + " | " + ], + "grants": [ + "grant_id | org | product | grant_reason | granted_by | extends | description | status", + " | | | default | none | none | | active", + " | | | legacy | | | Grandfathered on org-type default change: org-type default change () | active" + ], + "provisions": [ + "provision | pool | grant_id | status", + " | | | ended", + " | | | active" + ], + "positions": [ + "provision | ladder | product | status | ended", + " | | | ended | true", + " | | | active | false", + " | | | active | false" + ], + "transitions": [ + "pool | provision | ladder | from_rank | to_rank | transition_type | actor_type | actor | reason", + " | | | - | 0 | initiate | system | none | test fixture", + " | | | 0 | - | end | operator | | org-type default change (): grandfathered as legacy", + " | | | 0 | 0 | transfer | operator | | org-type default change (): grandfathered as legacy", + " | | | - | 0 | initiate | operator | | tier added to ladder " + ], + "entitlements": [ + "pool | resource_key | resource_limit", + " | plan_golden_seats | 1" + ] + } +} diff --git a/internal/server/testdata/plan_acts/renumbering-during-conferral/a-tier-delete.json b/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-delete.json similarity index 92% rename from internal/server/testdata/plan_acts/renumbering-during-conferral/a-tier-delete.json rename to internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-delete.json index 1d3ac8e3..13f44725 100644 --- a/internal/server/testdata/plan_acts/renumbering-during-conferral/a-tier-delete.json +++ b/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-delete.json @@ -44,7 +44,7 @@ "pool | provision | ladder | from_rank | to_rank | transition_type | actor_type | actor | reason", " | | | - | 0 | initiate | system | none | test fixture", " | | | 1 | - | end | operator | | org-type default change (): grandfathered as legacy", - " | | | 1 | 0 | downgrade | operator | | org-type default change (): grandfathered as legacy" + " | | | 1 | 1 | transfer | operator | | org-type default change (): grandfathered as legacy" ], "entitlements": [ "pool | resource_key | resource_limit", diff --git a/internal/server/testdata/plan_acts/renumbering-during-conferral/a-tier-removal.json b/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-removal.json similarity index 91% rename from internal/server/testdata/plan_acts/renumbering-during-conferral/a-tier-removal.json rename to internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-removal.json index 9c42a954..af57b090 100644 --- a/internal/server/testdata/plan_acts/renumbering-during-conferral/a-tier-removal.json +++ b/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-removal.json @@ -49,8 +49,8 @@ " | | | - | 0 | initiate | system | none | test fixture", " | | | - | 0 | initiate | operator | | test fixture", " | | | 1 | - | end | operator | | org-type default change (): grandfathered as legacy", - " | | | - | - | end | operator | | tier removal (Plus from Main)", - " | | | 1 | 0 | downgrade | operator | | org-type default change (): grandfathered as legacy" + " | | | 1 | 1 | transfer | operator | | org-type default change (): grandfathered as legacy", + " | | | - | - | end | operator | | tier removal (Plus from Main)" ], "entitlements": [ "pool | resource_key | resource_limit", diff --git a/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/.openspec.yaml b/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/.openspec.yaml new file mode 100644 index 00000000..53f62d70 --- /dev/null +++ b/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-10 diff --git a/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/proposal.md b/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/proposal.md new file mode 100644 index 00000000..99dd71cf --- /dev/null +++ b/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/proposal.md @@ -0,0 +1,28 @@ +## Why + +A conferral can record a tier change with the wrong type when an operator changes the same ladder's tiers at the same moment (#213). `core.confer` reads the incumbent's rank, the same rank again for the `end` row, and the new product's rank and shape through `core.product_conferral_shapes`, each in its own statement, and materializing transactions run at READ COMMITTED. The reorder, the tier delete and the tier removal renumber a ladder, and the tier add grows a product's shape, under the shared materialization lock with no lock a conferral holds, so one of them can commit between those reads. A grandfathered organization that keeps its product is then recorded as upgraded from rank 0 to rank 1, the tier-change ledger keeps the wrong row for good, and Tier changes and Recent activity name a product the organization never held. `TestTierReorder_CommitRacesDefaultChangeCommit` catches it about 1 run in 25 under load. + +## What Changes + +- The reorder's renumbering, the tier delete, the tier removal and the tier add open their transactions with `BeginRuleChange`, which takes the materialization lock in exclusive mode, instead of `BeginMaterializing`. Every conferral, grant act, reconcile, drain and floor restoration holds the lock in shared mode from its first statement, so each one finishes before the tier change or starts after it, and reads one order of the ladder. +- The default change and the reorder's per-organization enactments keep the shared lock. +- A tier change now waits for every materializing transaction in flight, up to the 5 s lock timeout, and new materializing transactions wait behind it while it runs. A tier change that times out answers as it answers a rule change in progress today: the removal with "Another entitlement set change is in progress. Try again.", the add, the delete and the reorder with their own failure copy. +- `docs/database-locks.md` and `docs/go-conventions.md` say which transactions take the exclusive lock. + +No schema migration. No change to `core.confer`. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `entitlements`: "A transaction that materializes a pool is ordered against a rule change" names the four tier changes as exclusive holders, says a materializing transaction may hold the lock in exclusive mode, and gains two scenarios ordering a tier change against a conferral. + +## Impact + +- Code: `internal/server/operator_plan_ladders.go` (four transaction openers), `internal/entitlements/tx.go` (comments). +- Tests: the plan-wide acts' concurrency tests (the renumbering race now waits instead of committing inside the conferral, and a tier add joins it; the removal-versus-default-change queue orders; the stale reorder candidate), the act goldens' rendezvous line for the four acts, and the comment of `TestTierReorder_CommitRacesDefaultChangeCommit`, which stops failing intermittently. +- Out of scope: the generic copy the add, the delete and the reorder answer on a lock timeout, where the requirement asks for the refusal sentence; it now reaches a request that waits behind any materializing transaction, not only a rule change. `core.confer` reading every rank once. The shape of the plan-wide acts' transactions (#187). diff --git a/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/specs/entitlements/spec.md b/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/specs/entitlements/spec.md new file mode 100644 index 00000000..9ea40c30 --- /dev/null +++ b/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/specs/entitlements/spec.md @@ -0,0 +1,63 @@ +## MODIFIED Requirements + +### Requirement: A transaction that materializes a pool is ordered against a rule change + +Every transaction that materializes a pool SHALL take the `entitlement_materialization` advisory lock as its first acquisition, before it takes any row: in shared mode, or in exclusive mode when the same transaction changes a rule or carries an operator's tier change. Every transaction that adds, changes, deactivates or reactivates an entitlement set rule, and every transaction of an operator act that changes a plan ladder's tiers (the tier add, the tier delete, the tier removal and the reorder's renumbering), SHALL take the same lock in exclusive mode as its first acquisition, before it takes any row. The rule-change transaction SHALL then take the entitlement set's row before any pool row. Any transaction that takes more than one pool row SHALL take them in ascending pool id order. Because the advisory lock is the first acquisition on both sides, no holder waits for something a would-be holder holds, so a rule change or a tier change and a recomputation are strictly ordered whichever begins first and neither can hold a row the other waits for. A conferral reads a ladder's ranks and a product's conferral shape in more than one statement, so this ordering is what makes it classify each tier change against one order of the ladder. + +Shared holders SHALL NOT block each other, so recomputations continue to run concurrently with one another. Each such transaction SHALL set a lock timeout, so a wait surfaces as a refusal an operator can read and retry rather than as a hang; a timed-out wait for the materialization lock SHALL refuse with "Another entitlement set change is in progress. Try again.", and a deadlock reported by the database SHALL surface as the same retryable refusal rather than as a server error. + +Possession SHALL be enforced rather than trusted. Before it writes anything, the materializer's apply step SHALL assert that its own transaction holds the rendezvous, by calling the one shared database assertion that queries the lock state for the calling backend, and SHALL refuse with a named error identifying the pool when it does not. Every transaction that adds, changes, deactivates or reactivates a rule SHALL call the same assertion in its exclusive form as its own first statement. A caller that forgets the rendezvous therefore refuses a conferral, an activation or a grant at once, instead of writing limits derived from a rule state another transaction has already replaced. Lock order is not asserted: a transaction that takes a pool row first surfaces its mistake as a lock timeout or a deadlock, which is a refused request rather than a wrong limit. Other advisory locks MAY be taken before the rendezvous, which stays safe because no rule-change or tier-change transaction takes any of them. + +Membership is decided by what a transaction does, not by which primitive it calls. The transactions bound by this requirement today are the rule-change commit and its drain, the tier add, the tier delete, the tier removal and the reorder's renumbering, the org-type default change and the reorder's per-organization enactments, grant issuance and grant revocation, the demonstration seed's materialization, and the subscription reconcile's suspend and resume paths. The rule-change commit and the four tier changes SHALL open through the exclusive opener and every other one through the shared opener, each covered by a test that drives the caller and asserts it does not refuse. The system SHALL provide one transaction opener for each side, so that a transaction which materializes a pool without opening through them is a defect that can be found by reading the code as well as by running it. + +#### Scenario: A conferral that begins first finishes before the rule change + +- **WHEN** a conferral transaction has taken the shared lock and a rule commit arrives +- **THEN** the rule commit waits for the exclusive lock until that conferral's transaction ends +- **AND** the conferral materializes against the rules that were live when it began + +#### Scenario: A conferral that begins during a rule change waits for it + +- **WHEN** a rule commit holds the exclusive lock and a conferral for a pool that is about to carry the set begins +- **THEN** the conferral waits before taking any row +- **AND** it materializes against the committed rule state, so no pool is left materialized from a rule state the commit replaced + +#### Scenario: Two recomputations do not block each other + +- **WHEN** two conferrals on different pools run at the same time with no rule change in flight +- **THEN** both hold the lock in shared mode and neither waits for the other + +#### Scenario: A wait that exceeds the timeout is a readable refusal + +- **WHEN** a transaction waits longer than the configured lock timeout for the materialization lock +- **THEN** it rolls back and the refusal reads "Another entitlement set change is in progress. Try again." +- **AND** no partial write survives + +#### Scenario: The reconcile's resume path is bound by the invariant + +- **WHEN** a subscription reconcile resumes a suspended provision and materializes its pool +- **THEN** its transaction takes the shared materialization lock before any row +- **AND** it cannot write limits derived from a rule state a rule commit has already replaced + +#### Scenario: A materializer that holds no rendezvous refuses + +- **WHEN** a transaction that did not take the materialization lock calls the materializer's apply step +- **THEN** it refuses with a named error identifying the pool and writes nothing + +#### Scenario: The reconcile's per-subscription lock follows the rendezvous + +- **WHEN** the subscription reconcile takes the shared materialization lock and then its per-subscription advisory lock +- **THEN** the assertion passes and the materialization proceeds + + +#### Scenario: A tier change waits for a conferral in flight + +- **WHEN** a conferral transaction holds the shared lock and has read a ladder's ranks, and a reorder, a tier delete, a tier removal or a tier add on that ladder arrives +- **THEN** the tier change waits for the exclusive lock until that conferral's transaction ends +- **AND** the conferral classifies its transitions against the ladder as it read it, so a handoff of one product between sources is recorded as a `transfer` + +#### Scenario: A conferral that begins during a tier change waits for it + +- **WHEN** a reorder, a tier delete, a tier removal or a tier add holds the exclusive lock and a conferral on that ladder begins +- **THEN** the conferral waits before taking any row +- **AND** it reads the ladder's ranks and the product's shape as the tier change committed them diff --git a/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/tasks.md b/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/tasks.md new file mode 100644 index 00000000..eadd8530 --- /dev/null +++ b/openspec/changes/archive/2026-10-11-tier-changes-against-conferrals/tasks.md @@ -0,0 +1,16 @@ +## 1. The exclusive rendezvous on the tier changes + +- [x] 1.1 `ReorderPlanLadderTiers`, `DeletePlanLadderTier`, `CommitPlanLadderTierRemoval` and `CreatePlanLadderTier` open their transactions with `entitlements.BeginRuleChange`; the default change's and the reorder's per-organization enactments keep `BeginMaterializing`. +- [x] 1.2 `internal/entitlements/tx.go`: the rendezvous comment and `RuleChange`'s say a change to a ladder's tiers takes the exclusive mode, and why. + +## 2. Tests + +- [x] 2.1 `TestRenumberingCommitsDuringAConferral` becomes `TestTierChangesWaitForAConferral`: each tier change queues on the exclusive lock behind the paused conferral and commits after it; a tier add case joins; the files record `transfer`. +- [x] 2.2 `TestTierRemovalAgainstDefaultChange`'s queued cases wait for the second request on the lock instead of on the pool row. +- [x] 2.3 `TestReorderEnactsAgainstTheRankZeroItSaved` commits the other renumbering between the reorder's commit and its enactment. +- [x] 2.4 The act goldens of the reorder, the delete, the removal and the add record the exclusive lock; `TestTierReorder_CommitRacesDefaultChangeCommit`'s comment names the lock that orders the renumbering against the conferral. + +## 3. Docs + +- [x] 3.1 `docs/database-locks.md`: the rendezvous row, the tier section and rule 1. +- [x] 3.2 `docs/go-conventions.md`: the transaction kinds paragraph. diff --git a/openspec/specs/entitlements/spec.md b/openspec/specs/entitlements/spec.md index 77548fd1..709c4b16 100644 --- a/openspec/specs/entitlements/spec.md +++ b/openspec/specs/entitlements/spec.md @@ -435,13 +435,13 @@ Pools whose provisions carrying the set are all suspended SHALL NOT be part of t ### Requirement: A transaction that materializes a pool is ordered against a rule change -Every transaction that materializes a pool SHALL take the `entitlement_materialization` advisory lock in shared mode as its first acquisition, before it takes any row, and every transaction that adds, changes, deactivates or reactivates an entitlement set rule SHALL take the same lock in exclusive mode as its first acquisition, before it takes any row. The rule-change transaction SHALL then take the entitlement set's row before any pool row. Any transaction that takes more than one pool row SHALL take them in ascending pool id order. Because the advisory lock is the first acquisition on both sides, no holder waits for something a would-be holder holds, so a rule change and a recomputation are strictly ordered whichever begins first and neither can hold a row the other waits for. +Every transaction that materializes a pool SHALL take the `entitlement_materialization` advisory lock as its first acquisition, before it takes any row: in shared mode, or in exclusive mode when the same transaction changes a rule or carries an operator's tier change. Every transaction that adds, changes, deactivates or reactivates an entitlement set rule, and every transaction of an operator act that changes a plan ladder's tiers (the tier add, the tier delete, the tier removal and the reorder's renumbering), SHALL take the same lock in exclusive mode as its first acquisition, before it takes any row. The rule-change transaction SHALL then take the entitlement set's row before any pool row. Any transaction that takes more than one pool row SHALL take them in ascending pool id order. Because the advisory lock is the first acquisition on both sides, no holder waits for something a would-be holder holds, so a rule change or a tier change and a recomputation are strictly ordered whichever begins first and neither can hold a row the other waits for. A conferral reads a ladder's ranks and a product's conferral shape in more than one statement, so this ordering is what makes it classify each tier change against one order of the ladder. Shared holders SHALL NOT block each other, so recomputations continue to run concurrently with one another. Each such transaction SHALL set a lock timeout, so a wait surfaces as a refusal an operator can read and retry rather than as a hang; a timed-out wait for the materialization lock SHALL refuse with "Another entitlement set change is in progress. Try again.", and a deadlock reported by the database SHALL surface as the same retryable refusal rather than as a server error. -Possession SHALL be enforced rather than trusted. Before it writes anything, the materializer's apply step SHALL assert that its own transaction holds the rendezvous, by calling the one shared database assertion that queries the lock state for the calling backend, and SHALL refuse with a named error identifying the pool when it does not. Every transaction that adds, changes, deactivates or reactivates a rule SHALL call the same assertion in its exclusive form as its own first statement. A caller that forgets the rendezvous therefore refuses a conferral, an activation or a grant at once, instead of writing limits derived from a rule state another transaction has already replaced. Lock order is not asserted: a transaction that takes a pool row first surfaces its mistake as a lock timeout or a deadlock, which is a refused request rather than a wrong limit. Other advisory locks MAY be taken before the rendezvous, which stays safe because no rule-change transaction takes any of them. +Possession SHALL be enforced rather than trusted. Before it writes anything, the materializer's apply step SHALL assert that its own transaction holds the rendezvous, by calling the one shared database assertion that queries the lock state for the calling backend, and SHALL refuse with a named error identifying the pool when it does not. Every transaction that adds, changes, deactivates or reactivates a rule SHALL call the same assertion in its exclusive form as its own first statement. A caller that forgets the rendezvous therefore refuses a conferral, an activation or a grant at once, instead of writing limits derived from a rule state another transaction has already replaced. Lock order is not asserted: a transaction that takes a pool row first surfaces its mistake as a lock timeout or a deadlock, which is a refused request rather than a wrong limit. Other advisory locks MAY be taken before the rendezvous, which stays safe because no rule-change or tier-change transaction takes any of them. -Membership is decided by what a transaction does, not by which primitive it calls. The transactions bound by this requirement today are the rule-change commit and its drain, the tier add and tier removal enactments, the org-type default change, grant issuance and grant revocation, the demonstration seed, and the subscription reconcile's suspend and resume paths; every one of them SHALL open through the shared opener in this change, each covered by a test that drives the caller and asserts it does not refuse. The system SHALL provide one transaction opener for each side, so that a transaction which materializes a pool without opening through them is a defect that can be found by reading the code as well as by running it. +Membership is decided by what a transaction does, not by which primitive it calls. The transactions bound by this requirement today are the rule-change commit and its drain, the tier add, the tier delete, the tier removal and the reorder's renumbering, the org-type default change and the reorder's per-organization enactments, grant issuance and grant revocation, the demonstration seed's materialization, and the subscription reconcile's suspend and resume paths. The rule-change commit and the four tier changes SHALL open through the exclusive opener and every other one through the shared opener, each covered by a test that drives the caller and asserts it does not refuse. The system SHALL provide one transaction opener for each side, so that a transaction which materializes a pool without opening through them is a defect that can be found by reading the code as well as by running it. #### Scenario: A conferral that begins first finishes before the rule change @@ -482,6 +482,18 @@ Membership is decided by what a transaction does, not by which primitive it call - **WHEN** the subscription reconcile takes the shared materialization lock and then its per-subscription advisory lock - **THEN** the assertion passes and the materialization proceeds +#### Scenario: A tier change waits for a conferral in flight + +- **WHEN** a conferral transaction holds the shared lock and has read a ladder's ranks, and a reorder, a tier delete, a tier removal or a tier add on that ladder arrives +- **THEN** the tier change waits for the exclusive lock until that conferral's transaction ends +- **AND** the conferral classifies its transitions against the ladder as it read it, so a handoff of one product between sources is recorded as a `transfer` + +#### Scenario: A conferral that begins during a tier change waits for it + +- **WHEN** a reorder, a tier delete, a tier removal or a tier add holds the exclusive lock and a conferral on that ladder begins +- **THEN** the conferral waits before taking any row +- **AND** it reads the ladder's ranks and the product's shape as the tier change committed them + ### Requirement: The materializer computes and applies as two steps sharing one fold The materializer SHALL be a compute step that folds a pool's active provisions and their active rules into the would-be state per resource key, and an apply step that reconciles the pool's stored rows to that state. Both steps SHALL share one fold, so there is exactly one answer to what a pool is entitled to and no second implementation can diverge from it. The fold SHALL carry each contribution's reduction policy and SHALL report, per resource key, the governing policy across the contributions that fund it, by the order stated in "The reduction policy governing a pool and key is the strongest across the rules funding it". The compute step SHALL take no writer.