Lock the pool before the obligation when settling a rule change's obligation
#196. When a rule-change drain failed on a pool, recordObligationFailure recorded the attempt through core.settle_obligation, which locked the obligation row and then the pool row. The drain and the commit lock the pool first, and both transactions hold only the shared materialization rendezvous, so a failure record and an overlapping drain could each hold the row the other waited for; Postgres aborted one, and an aborted drain counted the deadlock as one of the obligation's five attempts. Migration 00038 replaces core.settle_obligation with the pool lock first, right after the rendezvous assertion, for a settlement and a failure alike. Everything else is migration 19's body unchanged; the owner, core_writer's EXECUTE, SECURITY DEFINER and search_path are kept. Down restores migration 19's body, and with it the inversion. A caller that already holds the pool re-locks it without waiting; a call on an obligation another drain has just settled now waits for that drain's pool before it returns without writing. docs/database-locks.md describes the new order for the commit and the drain's failure record, and drops the inversion from "Where the paths differ". The plan acts' failure recorder needs no change: it updates one obligation row and takes no pool. Attempt accounting is unchanged. Retry's own lock order is #306. Tests: TestSettleObligationLocksPoolBeforeObligation holds the pool in one transaction and checks, for a pending failure and a pending success, that the function waits for the pool before it locks the obligation and that a settlement committed during the wait is not changed; and TestSettlementPoolFirstMigrationUpDownUp checks that Down restores the previous function with its privileges and Up brings the new one back. Break list: 10 records, 10 CAUGHT. Full suite on reset databases, on this commit over main: 5612 passed, 16 skipped, none failed.
This commit is contained in:
@@ -101,8 +101,8 @@ transactions run at READ COMMITTED.
|
||||
same lock) and the rule row inside `core.commit_rule_change`. At or below
|
||||
the sync cap the same transaction then takes each carrying pool in ascending
|
||||
`pool_id` order (`ListPoolsCarryingSet`): `settlePool` locks the pool row
|
||||
through `LockPools`, one pool per call, then the obligation row and the pool
|
||||
row again inside `core.settle_obligation`, once per act.
|
||||
through `LockPools`, one pool per call, then the pool row again and the
|
||||
obligation row inside `core.settle_obligation`, once per act.
|
||||
- **Drain** (`DrainChange`, called by the commit's own drain above the cap and
|
||||
by the poller's activity): one short transaction per owed pool, opened
|
||||
through `BeginMaterializing`. It takes the rendezvous, the pool row
|
||||
@@ -110,9 +110,8 @@ transactions run at READ COMMITTED.
|
||||
in the batch's order. A failed pool rolls back, and `recordObligationFailure`
|
||||
records the attempt in a second transaction, also opened as `Materializing`:
|
||||
the rendezvous, then, through `MarkObligationFailed` and
|
||||
`core.settle_obligation`, the obligation row of each owed act and after it
|
||||
the pool row. That is the reverse of the drain's pool-then-obligation order
|
||||
(#196).
|
||||
`core.settle_obligation`, the pool row before the obligation row of each
|
||||
owed act in batch order. Later calls reuse the held pool lock.
|
||||
|
||||
### Grant acts
|
||||
|
||||
@@ -636,7 +635,3 @@ transaction that allocates or releases the name.
|
||||
restoration and materialization in the same order.
|
||||
- **Advisory keys are built four ways** (#186). The keys follow the four
|
||||
conventions above.
|
||||
- **Recording a failed drain locks the obligation row before the pool row**
|
||||
(#196), where the drain and the commit take the pool first. The plan acts'
|
||||
failure recorder takes the obligation row alone, in a plain transaction, and
|
||||
no pool row, so it has no such order.
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
-- SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial
|
||||
-- SPDX-FileCopyrightText: 2025-2026 Christian Galo
|
||||
|
||||
-- +goose Up
|
||||
-- ---------------------------------------------------------------------------
|
||||
-- core.settle_obligation takes the pool row before the obligation row (#196).
|
||||
-- The drain and the rule-change commit lock the pool and then each
|
||||
-- obligation. The failure recorder calls this function holding only the
|
||||
-- rendezvous, and the function used to take the obligation first, so a
|
||||
-- recorder could hold an obligation while waiting for a pool that a drain
|
||||
-- held while the drain waited for that obligation. The pool lock now follows
|
||||
-- the rendezvous assertion, for success and failure alike. Everything else is
|
||||
-- 00019's Up body unchanged; CREATE OR REPLACE keeps the owner (core_owner)
|
||||
-- and core_writer's EXECUTE, which the statements after the body restate.
|
||||
-- Down restores 00019's body, and with it the inversion.
|
||||
-- ---------------------------------------------------------------------------
|
||||
-- +goose StatementBegin
|
||||
CREATE OR REPLACE FUNCTION core.settle_obligation(
|
||||
p_change_id UUID,
|
||||
p_pool_id UUID,
|
||||
p_effects JSONB,
|
||||
p_failure TEXT DEFAULT NULL,
|
||||
p_max_attempts INTEGER DEFAULT NULL
|
||||
) RETURNS INTEGER
|
||||
LANGUAGE plpgsql
|
||||
SECURITY DEFINER
|
||||
SET search_path = core, pg_temp
|
||||
AS $$
|
||||
DECLARE
|
||||
v_status VARCHAR(20);
|
||||
v_attempts INTEGER;
|
||||
v_org_id UUID;
|
||||
v_org_name VARCHAR(255);
|
||||
v_written INTEGER := 0;
|
||||
BEGIN
|
||||
PERFORM core.assert_rendezvous(FALSE);
|
||||
|
||||
PERFORM 1 FROM core.resource_pools rp WHERE rp.pool_id = p_pool_id FOR UPDATE;
|
||||
|
||||
SELECT o.status, o.attempts INTO v_status, v_attempts
|
||||
FROM core.entitlement_set_change_obligations o
|
||||
WHERE o.change_id = p_change_id AND o.pool_id = p_pool_id
|
||||
FOR UPDATE;
|
||||
IF NOT FOUND THEN
|
||||
RAISE EXCEPTION 'obligation_not_found' USING ERRCODE = '22023',
|
||||
DETAIL = 'change ' || p_change_id || ' owes nothing to pool ' || p_pool_id;
|
||||
END IF;
|
||||
IF v_status = 'settled' THEN
|
||||
RETURN 0;
|
||||
END IF;
|
||||
|
||||
IF p_failure IS NOT NULL THEN
|
||||
UPDATE core.entitlement_set_change_obligations o
|
||||
SET status = CASE WHEN p_max_attempts IS NULL OR o.attempts + 1 >= p_max_attempts THEN 'failed' ELSE 'pending' END,
|
||||
attempts = o.attempts + 1, last_attempt_at = NOW(), last_error = p_failure
|
||||
WHERE o.change_id = p_change_id AND o.pool_id = p_pool_id;
|
||||
RETURN 0;
|
||||
END IF;
|
||||
|
||||
SELECT rp.org_id, org.name INTO v_org_id, v_org_name
|
||||
FROM core.resource_pools rp
|
||||
JOIN core.organizations org ON org.org_id = rp.org_id
|
||||
WHERE rp.pool_id = p_pool_id;
|
||||
|
||||
INSERT INTO core.entitlement_set_change_effects (
|
||||
change_id, pool_id, org_id, org_name, resource_key, resource_label,
|
||||
limit_before, limit_after, granted_before, granted_after,
|
||||
usage_at_effect, over_usage, was_over_before, reduction_policy,
|
||||
governing_policy
|
||||
)
|
||||
SELECT p_change_id, p_pool_id, v_org_id, v_org_name,
|
||||
e->>'resource_key',
|
||||
COALESCE(k.display_name, e->>'resource_key'),
|
||||
(e->>'limit_before')::BIGINT, (e->>'limit_after')::BIGINT,
|
||||
(e->>'granted_before')::BOOLEAN, (e->>'granted_after')::BOOLEAN,
|
||||
(e->>'usage_at_effect')::BIGINT,
|
||||
COALESCE((e->>'over_usage')::BOOLEAN, FALSE),
|
||||
COALESCE((e->>'was_over_before')::BOOLEAN, FALSE),
|
||||
e->>'reduction_policy',
|
||||
e->>'governing_policy'
|
||||
FROM jsonb_array_elements(COALESCE(p_effects, '[]'::JSONB)) AS e
|
||||
LEFT JOIN core.resource_keys k ON k.resource_key = e->>'resource_key';
|
||||
GET DIAGNOSTICS v_written = ROW_COUNT;
|
||||
|
||||
UPDATE core.entitlement_set_change_obligations o
|
||||
SET status = 'settled', settled_at = NOW(), attempts = o.attempts + 1, last_attempt_at = NOW(), last_error = NULL
|
||||
WHERE o.change_id = p_change_id AND o.pool_id = p_pool_id;
|
||||
|
||||
RETURN v_written;
|
||||
END;
|
||||
$$;
|
||||
|
||||
ALTER FUNCTION core.settle_obligation(UUID, UUID, JSONB, TEXT, INTEGER) OWNER TO core_owner;
|
||||
GRANT EXECUTE ON FUNCTION core.settle_obligation(UUID, UUID, JSONB, TEXT, INTEGER) TO core_writer;
|
||||
-- +goose StatementEnd
|
||||
|
||||
-- +goose Down
|
||||
-- +goose StatementBegin
|
||||
CREATE OR REPLACE FUNCTION core.settle_obligation(
|
||||
p_change_id UUID,
|
||||
p_pool_id UUID,
|
||||
p_effects JSONB,
|
||||
p_failure TEXT DEFAULT NULL,
|
||||
p_max_attempts INTEGER DEFAULT NULL
|
||||
) RETURNS INTEGER
|
||||
LANGUAGE plpgsql
|
||||
SECURITY DEFINER
|
||||
SET search_path = core, pg_temp
|
||||
AS $$
|
||||
DECLARE
|
||||
v_status VARCHAR(20);
|
||||
v_attempts INTEGER;
|
||||
v_org_id UUID;
|
||||
v_org_name VARCHAR(255);
|
||||
v_written INTEGER := 0;
|
||||
BEGIN
|
||||
PERFORM core.assert_rendezvous(FALSE);
|
||||
|
||||
SELECT o.status, o.attempts INTO v_status, v_attempts
|
||||
FROM core.entitlement_set_change_obligations o
|
||||
WHERE o.change_id = p_change_id AND o.pool_id = p_pool_id
|
||||
FOR UPDATE;
|
||||
IF NOT FOUND THEN
|
||||
RAISE EXCEPTION 'obligation_not_found' USING ERRCODE = '22023',
|
||||
DETAIL = 'change ' || p_change_id || ' owes nothing to pool ' || p_pool_id;
|
||||
END IF;
|
||||
IF v_status = 'settled' THEN
|
||||
RETURN 0;
|
||||
END IF;
|
||||
|
||||
PERFORM 1 FROM core.resource_pools rp WHERE rp.pool_id = p_pool_id FOR UPDATE;
|
||||
|
||||
IF p_failure IS NOT NULL THEN
|
||||
UPDATE core.entitlement_set_change_obligations o
|
||||
SET status = CASE WHEN p_max_attempts IS NULL OR o.attempts + 1 >= p_max_attempts THEN 'failed' ELSE 'pending' END,
|
||||
attempts = o.attempts + 1, last_attempt_at = NOW(), last_error = p_failure
|
||||
WHERE o.change_id = p_change_id AND o.pool_id = p_pool_id;
|
||||
RETURN 0;
|
||||
END IF;
|
||||
|
||||
SELECT rp.org_id, org.name INTO v_org_id, v_org_name
|
||||
FROM core.resource_pools rp
|
||||
JOIN core.organizations org ON org.org_id = rp.org_id
|
||||
WHERE rp.pool_id = p_pool_id;
|
||||
|
||||
INSERT INTO core.entitlement_set_change_effects (
|
||||
change_id, pool_id, org_id, org_name, resource_key, resource_label,
|
||||
limit_before, limit_after, granted_before, granted_after,
|
||||
usage_at_effect, over_usage, was_over_before, reduction_policy,
|
||||
governing_policy
|
||||
)
|
||||
SELECT p_change_id, p_pool_id, v_org_id, v_org_name,
|
||||
e->>'resource_key',
|
||||
COALESCE(k.display_name, e->>'resource_key'),
|
||||
(e->>'limit_before')::BIGINT, (e->>'limit_after')::BIGINT,
|
||||
(e->>'granted_before')::BOOLEAN, (e->>'granted_after')::BOOLEAN,
|
||||
(e->>'usage_at_effect')::BIGINT,
|
||||
COALESCE((e->>'over_usage')::BOOLEAN, FALSE),
|
||||
COALESCE((e->>'was_over_before')::BOOLEAN, FALSE),
|
||||
e->>'reduction_policy',
|
||||
e->>'governing_policy'
|
||||
FROM jsonb_array_elements(COALESCE(p_effects, '[]'::JSONB)) AS e
|
||||
LEFT JOIN core.resource_keys k ON k.resource_key = e->>'resource_key';
|
||||
GET DIAGNOSTICS v_written = ROW_COUNT;
|
||||
|
||||
UPDATE core.entitlement_set_change_obligations o
|
||||
SET status = 'settled', settled_at = NOW(), attempts = o.attempts + 1, last_attempt_at = NOW(), last_error = NULL
|
||||
WHERE o.change_id = p_change_id AND o.pool_id = p_pool_id;
|
||||
|
||||
RETURN v_written;
|
||||
END;
|
||||
$$;
|
||||
|
||||
ALTER FUNCTION core.settle_obligation(UUID, UUID, JSONB, TEXT, INTEGER) OWNER TO core_owner;
|
||||
GRANT EXECUTE ON FUNCTION core.settle_obligation(UUID, UUID, JSONB, TEXT, INTEGER) TO core_writer;
|
||||
-- +goose StatementEnd
|
||||
@@ -0,0 +1,87 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial
|
||||
// SPDX-FileCopyrightText: 2025-2026 Christian Galo
|
||||
|
||||
package db_test
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"testing"
|
||||
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/db"
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/testkit/dbtest"
|
||||
)
|
||||
|
||||
const (
|
||||
settlementPoolFirstVersion = 38
|
||||
settlementPoolFirstPrevious = 37
|
||||
settlementFunction = "core.settle_obligation(uuid,uuid,jsonb,text,integer)"
|
||||
)
|
||||
|
||||
// TestSettlementPoolFirstMigrationUpDownUp checks the restored body, the
|
||||
// function's privileges at each step and runtime calls after both Ups.
|
||||
func TestSettlementPoolFirstMigrationUpDownUp(t *testing.T) {
|
||||
scratch := dbtest.Connect(t, dbtest.Scratch(t))
|
||||
core := db.BaseSources()[0]
|
||||
queries := dbtest.On(t, scratch)
|
||||
dbtest.UpTo(t, scratch, core, settlementPoolFirstPrevious)
|
||||
previous := queries.String(`SELECT pg_get_functiondef($1::regprocedure)`, settlementFunction)
|
||||
privileges := settlementFunctionPrivileges(queries)
|
||||
|
||||
for _, step := range []struct {
|
||||
name string
|
||||
up bool
|
||||
}{
|
||||
{name: "first up", up: true},
|
||||
{name: "down"},
|
||||
{name: "second up", up: true},
|
||||
} {
|
||||
dbtest.ApplyVersion(t, scratch, core, settlementPoolFirstVersion, step.up)
|
||||
if got := settlementFunctionPrivileges(queries); got != privileges {
|
||||
t.Errorf("%s: settlement function privileges changed: %s, want %s", step.name, got, privileges)
|
||||
}
|
||||
if step.up {
|
||||
checkRuntimeSettlement(t, scratch, step.name, nil)
|
||||
checkRuntimeSettlement(t, scratch, step.name, "simulated")
|
||||
} else if got := queries.String(`SELECT pg_get_functiondef($1::regprocedure)`, settlementFunction); got != previous {
|
||||
t.Error("down did not restore the previous settlement function")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// settlementFunctionPrivileges includes the signature and execution settings
|
||||
// beside the owner and ACL that CREATE OR REPLACE must preserve.
|
||||
func settlementFunctionPrivileges(queries *dbtest.Q) string {
|
||||
return queries.String(`SELECT json_build_object(
|
||||
'owner', pg_get_userbyid(proowner), 'acl', proacl::text,
|
||||
'definer', prosecdef, 'config', proconfig,
|
||||
'arguments', pg_get_function_arguments(oid), 'result', pg_get_function_result(oid))::text
|
||||
FROM pg_proc WHERE oid = $1::regprocedure`, settlementFunction)
|
||||
}
|
||||
|
||||
// checkRuntimeSettlement writes a pending obligation directly because the
|
||||
// migration's function, rather than rule authoring, is the subject. Each call
|
||||
// gets its own rolled-back fixture and runs as the application's role.
|
||||
func checkRuntimeSettlement(t *testing.T, database *sql.DB, step string, failure any) {
|
||||
t.Helper()
|
||||
queries := dbtest.Tx(t, database)
|
||||
defer queries.Tx().Rollback()
|
||||
queries.Exec(sharedRendezvous)
|
||||
person := fxPerson(t, queries.Tx())
|
||||
org := fxOrg(t, queries.Tx(), person)
|
||||
pool := fxPool(t, queries.Tx(), org, "default")
|
||||
set := queries.ID(`INSERT INTO core.entitlement_sets (name) VALUES ($1) RETURNING set_id`, dbtest.Name("Settlement "))
|
||||
key := dbtest.Name("settlement-")
|
||||
queries.Exec(`INSERT INTO core.resource_keys (resource_key, display_name, unit) VALUES ($1, $1, 'items')`, key)
|
||||
rule := queries.ID(`INSERT INTO core.entitlement_set_rules
|
||||
(set_id, rule_type, resource_key, resource_value, resource_per_unit, stacking_policy)
|
||||
VALUES ($1, 'limit', $2, 5, false, 'additive') RETURNING rule_id`, set, key)
|
||||
change := queries.ID(`INSERT INTO core.entitlement_set_changes
|
||||
(set_id, set_name, change_kind, rule_id, rule_after, actor_type, sync_path, effective_at)
|
||||
VALUES ($1, 'Settlement', 'rule_added', $2, '{}'::jsonb, 'system', 'deferred', NOW()) RETURNING change_id`, set, rule)
|
||||
queries.Exec(`INSERT INTO core.entitlement_set_change_obligations (change_id, pool_id) VALUES ($1, $2)`, change, pool)
|
||||
asRole(t, queries.Tx(), db.RuntimeGroupRole, func() {
|
||||
if err := queries.Attempt(`SELECT core.settle_obligation($1, $2, '[]'::jsonb, $3::text, 5)`, change, pool, failure); err != nil {
|
||||
t.Errorf("%s: runtime settlement returned %v", step, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,208 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial
|
||||
// SPDX-FileCopyrightText: 2025-2026 Christian Galo
|
||||
|
||||
package entitlements_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
|
||||
"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/dbtest"
|
||||
)
|
||||
|
||||
// obligationLockCase names the call waiting behind a pool holder and the
|
||||
// state it must leave after the holder settles the obligation.
|
||||
type obligationLockCase struct {
|
||||
name string
|
||||
failure bool
|
||||
wantStatus string
|
||||
wantAttempts int
|
||||
wantError sql.NullString
|
||||
wantEffects int64
|
||||
}
|
||||
|
||||
// obligationLockState includes the timestamps a late call must not rewrite.
|
||||
type obligationLockState struct {
|
||||
status string
|
||||
attempts int
|
||||
lastError sql.NullString
|
||||
lastAttempt sql.NullTime
|
||||
settled sql.NullTime
|
||||
}
|
||||
|
||||
// String prints the state with readable timestamps for a failure message.
|
||||
func (state obligationLockState) String() string {
|
||||
at := func(value sql.NullTime) string {
|
||||
if !value.Valid {
|
||||
return "NULL"
|
||||
}
|
||||
return value.Time.Format(time.RFC3339Nano)
|
||||
}
|
||||
return fmt.Sprintf("{status %s, attempts %d, last_error %v, last_attempt_at %s, settled_at %s}",
|
||||
state.status, state.attempts, state.lastError, at(state.lastAttempt), at(state.settled))
|
||||
}
|
||||
|
||||
// TestSettleObligationLocksPoolBeforeObligation observes the waiting call
|
||||
// before probing its obligation; an obligation-first order would hold that row
|
||||
// during the wait.
|
||||
func TestSettleObligationLocksPoolBeforeObligation(t *testing.T) {
|
||||
for _, row := range []obligationLockCase{
|
||||
{name: "pending failure", failure: true, wantStatus: "settled", wantAttempts: 1, wantError: sql.NullString{}},
|
||||
{name: "pending success", wantStatus: "settled", wantAttempts: 1, wantError: sql.NullString{}, wantEffects: 0},
|
||||
} {
|
||||
t.Run(row.name, func(t *testing.T) { checkObligationLockOrder(t, row) })
|
||||
}
|
||||
}
|
||||
|
||||
// pendingLockObligation commits a rule change without its drain, so both
|
||||
// test transactions can read the same pending obligation.
|
||||
func pendingLockObligation(t *testing.T, ctx context.Context, database *sql.DB) (changeID, poolID, key string) {
|
||||
t.Helper()
|
||||
setup, err := entitlements.BeginRuleChange(ctx, database)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer setup.Rollback()
|
||||
queries := entitlements.New(setup)
|
||||
org := setupTestOrg(t, ctx, setup)
|
||||
product := createTestProduct(t, ctx, setup, dbtest.Name("Lock order "), 5, false)
|
||||
createGrantAndProvision(t, ctx, queries, org, product.productID, 1)
|
||||
rule := firstActiveRule(t, ctx, queries, product.setID)
|
||||
change, err := entitlements.CommitRuleChangeTx(ctx, queries, entitlements.CommitRuleChangeInput{
|
||||
SetID: product.setID, ChangeKind: entitlements.ChangeKindRuleModified,
|
||||
RuleID: rule.RuleID, ActorType: entitlements.ActorTypeSystem,
|
||||
Rule: entitlements.RuleFields{ResourceValue: sql.NullInt64{Int64: 7, Valid: true}},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := setup.Commit(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return change.ChangeID, org.pool.PoolID, rule.ResourceKey.String
|
||||
}
|
||||
|
||||
// readLockObligation reads only the fixture's obligation, on either an open
|
||||
// transaction or the database after both transactions have committed.
|
||||
func readLockObligation(t *testing.T, queries *dbtest.Q, changeID, poolID string) obligationLockState {
|
||||
t.Helper()
|
||||
var state obligationLockState
|
||||
queries.Scan([]any{&state.status, &state.attempts, &state.lastError, &state.lastAttempt, &state.settled},
|
||||
`SELECT status, attempts, last_error, last_attempt_at, settled_at
|
||||
FROM core.entitlement_set_change_obligations WHERE change_id = $1 AND pool_id = $2`, changeID, poolID)
|
||||
return state
|
||||
}
|
||||
|
||||
// checkObligationLockOrder holds the pool while the second call waits,
|
||||
// settles under that hold, and checks that the second call writes nothing.
|
||||
func checkObligationLockOrder(t *testing.T, row obligationLockCase) {
|
||||
t.Helper()
|
||||
database := dbtest.Open(t)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer cancel()
|
||||
changeID, poolID, key := pendingLockObligation(t, ctx, database)
|
||||
payload, err := json.Marshal([]map[string]any{{
|
||||
"resource_key": key, "limit_before": 5, "limit_after": 7,
|
||||
"usage_at_effect": 0, "reduction_policy": "defer", "governing_policy": "defer",
|
||||
}})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
holder, err := entitlements.BeginMaterializing(ctx, database)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer holder.Rollback()
|
||||
holderQueries := dbtest.On(t, holder)
|
||||
holderQueries.Exec("SET LOCAL ROLE " + db.RuntimeGroupRole)
|
||||
if _, err := entitlements.New(holder).LockPools(ctx, poolID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
waiter, err := entitlements.BeginMaterializing(ctx, database)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer waiter.Rollback()
|
||||
waiterQueries := dbtest.On(t, waiter)
|
||||
waiterQueries.Exec("SET LOCAL ROLE " + db.RuntimeGroupRole)
|
||||
pid := int(waiterQueries.Int(`SELECT pg_backend_pid()`))
|
||||
finished := make(chan struct{})
|
||||
var callErr error
|
||||
go func() {
|
||||
defer close(finished)
|
||||
queries := entitlements.New(waiter)
|
||||
if row.failure {
|
||||
callErr = queries.MarkObligationFailed(ctx, entitlements.MarkObligationFailedParams{
|
||||
ChangeID: changeID, PoolID: poolID, Failure: "simulated", MaxAttempts: 5,
|
||||
})
|
||||
} else {
|
||||
_, callErr = queries.SettleObligation(ctx, entitlements.SettleObligationParams{
|
||||
ChangeID: changeID, PoolID: poolID, Effects: payload,
|
||||
})
|
||||
}
|
||||
}()
|
||||
defer func() {
|
||||
cancel()
|
||||
holder.Rollback()
|
||||
<-finished
|
||||
}()
|
||||
dbtest.AwaitLockWait(t, database, dbtest.Backend{PID: pid, Abort: func() error {
|
||||
select {
|
||||
case <-finished:
|
||||
return fmt.Errorf("%s: settlement did not wait for the pool: %v", row.name, callErr)
|
||||
default:
|
||||
return nil
|
||||
}
|
||||
}})
|
||||
holderQueries.Exec("SAVEPOINT obligation_probe")
|
||||
_, err = holder.ExecContext(ctx, `SELECT 1 FROM core.entitlement_set_change_obligations
|
||||
WHERE change_id = $1 AND pool_id = $2 FOR UPDATE NOWAIT`, changeID, poolID)
|
||||
if err != nil {
|
||||
var pgErr *pgconn.PgError
|
||||
if errors.As(err, &pgErr) && pgErr.Code == "55P03" {
|
||||
t.Fatalf("%s: obligation locked before pool: %v", row.name, err)
|
||||
}
|
||||
t.Fatalf("%s: probe the obligation: %v", row.name, err)
|
||||
}
|
||||
holderQueries.Exec("ROLLBACK TO SAVEPOINT obligation_probe")
|
||||
if _, err := entitlements.New(holder).SettleObligation(ctx, entitlements.SettleObligationParams{
|
||||
ChangeID: changeID, PoolID: poolID, Effects: []byte(`[]`),
|
||||
}); err != nil {
|
||||
t.Fatalf("%s: holder settlement returned an error: %v", row.name, err)
|
||||
}
|
||||
heldState := readLockObligation(t, holderQueries, changeID, poolID)
|
||||
if err := holder.Commit(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
<-finished
|
||||
if callErr != nil {
|
||||
t.Errorf("%s: settlement returned an error: %v", row.name, callErr)
|
||||
return
|
||||
}
|
||||
if err := waiter.Commit(); err != nil {
|
||||
t.Fatalf("%s: settlement returned an error: %v", row.name, err)
|
||||
}
|
||||
queries := dbtest.On(t, database)
|
||||
want := obligationLockState{
|
||||
status: row.wantStatus, attempts: row.wantAttempts, lastError: row.wantError,
|
||||
lastAttempt: heldState.lastAttempt, settled: heldState.settled,
|
||||
}
|
||||
if got := readLockObligation(t, queries, changeID, poolID); got != want {
|
||||
t.Errorf("%s: late settlement changed the obligation: got %v, want %v", row.name, got, want)
|
||||
}
|
||||
if !row.failure {
|
||||
if count := queries.Int(`SELECT count(*) FROM core.entitlement_set_change_effects
|
||||
WHERE change_id = $1 AND pool_id = $2`, changeID, poolID); count != row.wantEffects {
|
||||
t.Errorf("%s: late settlement wrote %d effects, want %d", row.name, count, row.wantEffects)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,2 @@
|
||||
schema: spec-driven
|
||||
created: 2026-10-30
|
||||
@@ -0,0 +1,62 @@
|
||||
## The paths at HEAD
|
||||
|
||||
The inspected revision is `8c5bb1d3`. The current `core.settle_obligation` body is migration 19's Up body, not migration 18's original or migration 19's Down body. The table below includes existing-row locks, inserts and their foreign-key checks. Ordinary reads of obligation status and work lists take no row lock. References are to that revision.
|
||||
|
||||
| Path | Locks and writes, in order |
|
||||
|---|---|
|
||||
| Rule-change enumeration, `core.commit_rule_change` (`internal/db/migrations/00018_entitlement_set_changes.sql:454`) | Exclusive rendezvous, set and rule rows, new change and obligation inserts. Each obligation's foreign key takes key-share on its pool. It does not lock an existing obligation. |
|
||||
| Atomic rule-change settlement, `settleCarryingPools` (`internal/entitlements/rule_change.go:496`) | In that same exclusive transaction, carrying pools in ascending id order, one pool per `settlePool`; pool `FOR UPDATE`, then each obligation `FOR UPDATE` in the batch's resource-key order, then the already-held pool again. The inserted obligations are private until commit. |
|
||||
| Deferred request drain and poller, `DrainChange` / `drainPoolUnderActs` (`internal/entitlements/rule_change.go:850`, `:924`) | Shared rendezvous; status reads without row locks; one pool `FOR UPDATE` through `settlePool` / `LockPools` (`:609`, `:614`); each owed obligation `FOR UPDATE` in batch order, then the already-held pool again. Each pool commits separately. The test-only `DrainObligation` calls this same body (`internal/entitlements/export_test.go:48`). |
|
||||
| Rule-change failure record, `recordObligationFailure` (`internal/entitlements/rule_change.go:988`) | A separate shared-rendezvous transaction; for each owed act in batch order, `MarkObligationFailed` calls the function: obligation `FOR UPDATE`, then pool `FOR UPDATE`. Later calls reuse the pool lock. |
|
||||
| Direct settlement or failure call, `core.settle_obligation` (`internal/db/migrations/00019_effect_governing_policy.sql:29`; query wrappers at `internal/entitlements/queries/entitlement_set_changes.sql:32`, `:42`) | Assert rendezvous; obligation `FOR UPDATE`; return for an already settled row; otherwise pool `FOR UPDATE`. Failure updates the obligation. Success inserts effects, whose foreign keys check the already-held pool and obligation, then updates the obligation. The function is executable by `core_writer`, so its order matters independently of Go's caller. |
|
||||
| Rule-change Retry, `RequeueFailedObligations` (`internal/entitlements/queries/entitlement_set_changes.sql:104`; handler at `internal/server/operator_entitlement_recompute.go:84`) | One autocommit UPDATE, no rendezvous or pool lock. It locks failed obligations for a set in an unspecified order. Dispatch happens after the statement commits. |
|
||||
| Plan-act listing, `ListPlanActBatch` / `core.write_plan_act_obligations` (`internal/entitlements/plan_act_drain.go:379`; `internal/db/migrations/00028_plan_act_obligations.sql:337`) | Shared rendezvous; all batch pools `FOR KEY SHARE` in ascending pool id; older open obligations `FOR UPDATE` in ascending pool id; supersession updates; new inserts in org-type and org-id order. Pool foreign-key checks reuse the key-share locks. Concurrent inserts may also wait on uniqueness checks. |
|
||||
| Plan-act move, `EnactObligation` (`internal/entitlements/plan_act_move.go:76`, `:94`; `internal/entitlements/plan_act_runner.go:141`) | Shared rendezvous; pool `FOR UPDATE`; obligation `FOR UPDATE` through `core.lock_plan_act_obligation` (`internal/db/migrations/00032_lock_plan_act_obligation.sql:24`); conferral/end/restoration reuse that pool lock; `core.settle_plan_act_obligation` updates the held obligation. This is both the request and background move. |
|
||||
| Direct plan-act lock or settlement function (`internal/db/migrations/00032_lock_plan_act_obligation.sql:24`; `internal/db/migrations/00028_plan_act_obligations.sql:395`) | The lock function takes just the obligation `FOR UPDATE`. The settlement function asserts the rendezvous and updates just the pending obligation. Neither acquires a pool itself; the production move acquired it first. |
|
||||
| Plan-act failure record (`internal/entitlements/plan_act_drain.go:300`; `internal/db/migrations/00028_plan_act_obligations.sql:421`) | Plain transaction, no rendezvous; UPDATE of one pending obligation; no later pool acquisition. It changes no foreign-key column, so the existing pool reference does not acquire a pool lock. |
|
||||
| Plan-act Retry (`internal/entitlements/plan_act_retry.go:60`; `internal/db/migrations/00028_plan_act_obligations.sql:457`) | Shared rendezvous; insert Retry act and detail; failed obligations of still-owned types `FOR UPDATE` in ascending pool id; update them. No pool lock. Its subsequent listing and moves use separate transactions. |
|
||||
|
||||
These are the production writers and locking reads of the two obligation tables. No production path deletes their rows. Grant acts, subscription reconciliation, tier add/removal and pool conferral functions acquire pools but no obligation rows themselves; when called by a plan-act move they reuse its pool lock. A default change's and reorder's catalog transactions take the exclusive rendezvous and no pool or obligation rows. No transaction here takes both kinds of obligation row.
|
||||
|
||||
The rule-change obligation's pool foreign key is in `internal/db/migrations/00018_entitlement_set_changes.sql:141`, the effect's pool foreign key at `:178` and its obligation foreign key at `:181`. The plan-act pool foreign key is in `internal/db/migrations/00028_plan_act_obligations.sql:81`. There is no trigger on either obligation table that adds another pool acquisition. These paths agree with the relevant model rules: `docs/models/entitlements.md`, invariant 15; `docs/models/acts.md`, invariants 12–14 and 17; `docs/models/resource-pools.md`, pool identity and retention.
|
||||
|
||||
## Decision: put the existing pool lock first in the function
|
||||
|
||||
#196 is confirmed. A drain holds pool P and waits for obligation O. Its concurrent failure recorder holds O and waits for P. The shared rendezvous admits both. If PostgreSQL aborts the recorder, its attempt is lost; if it aborts the drain, the drain tries to record that error as an attempt. Neither leaves a partial write (`internal/entitlements/rule_change.go:944`).
|
||||
|
||||
Move the pool `PERFORM ... FOR UPDATE` immediately after `assert_rendezvous(FALSE)` in `core.settle_obligation`, before its first obligation read. Keep the remaining body unchanged, including the settled check, the missing-obligation error, effects and `governing_policy`, and failure threshold. As a consequence of this placement, a settled call waits for the pool too, bounded by the 5 s lock timeout, then still returns zero without writing. That wait is not a requirement: an unlocked status check that returns before acquiring either row would also be safe. An absent pool locks nothing; the existing missing-obligation error remains the answer. The obligation's foreign key prevents its pool from disappearing while the obligation exists.
|
||||
|
||||
The alternatives are:
|
||||
|
||||
| Option | Reason for the choice |
|
||||
|---|---|
|
||||
| `LockPools` in Go before `MarkObligationFailed` | Fixes this Go caller, but leaves the callable database function's obligation-before-pool order intact. Every other direct caller would still need to know to compensate. |
|
||||
| Pool first inside `core.settle_obligation` | Chosen. It reorders a lock the function already takes and covers both query wrappers and direct callers. It adds no API, lock family or error policy. Go settlement keeps its earlier `LockPools`, because materialization must also run under the pool lock. |
|
||||
| Failure recording without a pool lock | The failure update itself needs only its obligation, as the plan-act recorder demonstrates. However, simply skipping this lock leaves direct successful settlement's inversion, and a separate failure function or conditional ordering adds another contract. Removing that serialization can be considered separately; this fix does not foreclose it. |
|
||||
|
||||
Against the inventory: the failure recorder and a drain on the same pool now serialize before either takes an obligation. Atomic commits still exclude both at the rendezvous; their new obligations cannot be acquired by another transaction before commit. A caller that holds the pool re-locks it without waiting, including inside the atomic path's savepoint. Plan-act listing and moves may block at the pool, but neither then asks for a rule-change obligation held by the waiter. Plan-act failure and Retry never acquire a pool after their obligation. Pool-only paths never ask for either obligation. Rule-change Retry takes no pool, so the change introduces no return edge from its obligation to a pool.
|
||||
|
||||
`docs/database-locks.md` must describe the failure record as shared rendezvous, pool, then obligations in batch order, with subsequent calls reusing the pool. Its atomic-commit description must also stop describing the function as obligation then pool, and "Where the paths differ" must lose the failed-drain inversion bullet. This changes no advisory key or helper convention from #186.
|
||||
|
||||
The plan-act failure record has neither this inversion nor another lock-order inversion: it holds one obligation and requests nothing after it. A move can wait for it, but it cannot wait back on that move's pool or rendezvous.
|
||||
|
||||
## Failure paths
|
||||
|
||||
| Path | Today | After this change |
|
||||
|---|---|---|
|
||||
| Failure record overlaps a drain on its pool | The recorder can hold the obligation while it waits for the pool, and the drain holds the pool while it waits for that obligation; PostgreSQL aborts one. | The recorder waits at the pool without the obligation; after the drain commits, it reads the obligation settled and writes nothing. |
|
||||
| Failure record commits first | It records an attempt; a later drain can settle. | Same writes, with the drain waiting at the pool. Successful settlement still increments attempts and clears the error. |
|
||||
| Atomic or deferred successful settlement, including direct calls | The function takes the obligation before its pool; Go normally holds the pool already. | The function takes the pool first; effects and status writes are unchanged. |
|
||||
| Already settled or missing obligation | Settled returns zero without a pool lock; missing raises `obligation_not_found`. | Pool acquisition precedes both checks; return values and writes are unchanged. |
|
||||
| Failure recorder itself cannot begin, lock, write or commit | The attempt transaction rolls back; the drain returns the cause joined with the recording error. | Same. A wait at the pool still obeys the materializer's lock timeout. |
|
||||
| Other drain failure or commit error | A successful recorder counts it toward five attempts, including deadlocks and lock timeouts. | Same policy. No attempt or failed row is reset by this migration. |
|
||||
|
||||
The attempt policy is unchanged. Whether transient database errors should consume attempts, and the order in which Retry locks a set's failed obligations, are separate issues.
|
||||
|
||||
## Migration and proof
|
||||
|
||||
Add the next core migration, provisionally `00038_settlement_locks_pool_first.sql`, settling its number at landing. Follow `docs/database-management.md`: schema-qualified replacement, `StatementBegin` / `StatementEnd` around each complete Up and Down section, same signature and defaults, `SECURITY DEFINER`, `search_path = core, pg_temp`, owner `core_owner`, runtime execution through `core_writer`, no PUBLIC execution. Up changes only the lock placement from migration 19's Up body. Down restores that exact body, including `governing_policy`; it also restores the bug. There is no data rewrite or deployment-specific repair. Regenerate through `make sqlc-generate`; unchanged query signatures should leave generated output unchanged.
|
||||
|
||||
The decisive test holds the pool in transaction A and starts settlement or failure recording in transaction B, both under the shared rendezvous. `dbtest.AwaitLockWait` observes B waiting. While B waits, A probes the obligation with `FOR UPDATE NOWAIT` inside a savepoint: it must succeed. Under the old function B already holds the obligation, so this assertion fails deterministically without relying on PostgreSQL choosing a particular deadlock victim. A settles with an empty diff and commits; B completes as a no-op. Both rows start pending, and B's success call carries one effect so removing the settled guard produces an observable write. Use fixture-specific ids, bounded contexts, a completion channel, an abort probe for an early return, and cleanup that releases holders and joins goroutines on every exit. No sleeps establish an interleaving.
|
||||
|
||||
The migration test uses `dbtest.Scratch`, captures the old function definition and privileges, checks exact restoration by Down and privileges after each step, and exercises runtime execution after both Ups. Breaks of Down's restoration, Up's and Down's owner and grants, and Up's obligation read prove those checks. Existing effect goldens, enclosure and attempt-budget tests keep their results. No Copy table is needed: no user-visible string changes.
|
||||
@@ -0,0 +1,25 @@
|
||||
## Why
|
||||
|
||||
Recording a rule-change drain's failure takes the obligation row before the pool row. A concurrent drain takes the pool before the obligation. Both hold the shared materialization rendezvous, so neither excludes the other: each can hold the row the other needs. PostgreSQL aborts one transaction, and a drain aborted this way can consume an obligation's attempt budget. This is #196.
|
||||
|
||||
## What Changes
|
||||
|
||||
- Move the existing pool lock in `core.settle_obligation` before its obligation lock, for both success and failure. A new core migration replaces the function; its signature, writes, privileges and attempt policy stay the same.
|
||||
- Rewrite the rule-change commit and drain passages in `docs/database-locks.md` to describe the resulting order, including the failure record, and remove the obsolete inversion from "Where the paths differ".
|
||||
- Test the order with two open transactions and an observed lock wait, including a late failure after settlement, and test the migration up, down and up again.
|
||||
|
||||
## Capabilities
|
||||
|
||||
### New Capabilities
|
||||
|
||||
None.
|
||||
|
||||
### Modified Capabilities
|
||||
|
||||
- `entitlements`: add the requirement that settlement and failure recording lock the pool before a rule-change obligation.
|
||||
|
||||
## Impact
|
||||
|
||||
The implementation changes one core function, its documentation and tests. No query signature, provider schema, route, workflow, golden or user-visible string changes. A call on an already settled obligation will wait for the pool before returning without writing.
|
||||
|
||||
The plan-act failure recorder needs no change: it updates one obligation and takes no pool or rendezvous lock. Attempt accounting, Retry's obligation order and #186's shared lock helpers and advisory-key conventions remain outside this change.
|
||||
@@ -0,0 +1,18 @@
|
||||
## ADDED Requirements
|
||||
|
||||
### Requirement: Rule-change settlement locks the pool before the obligation
|
||||
|
||||
The database function that settles a rule-change obligation or records its failure SHALL acquire the pool row before the obligation row, after asserting the materialization rendezvous. This order SHALL apply to both success and failure calls. A call that waited for the pool SHALL leave unchanged an obligation that another transaction settled during the wait.
|
||||
|
||||
#### Scenario: A failure record waits behind a drain
|
||||
|
||||
- **WHEN** a drain holds a pool row and another transaction starts recording a failure on an obligation for that pool
|
||||
- **THEN** the failure recorder waits for the pool without holding the obligation row
|
||||
- **AND** the drain can acquire the obligation and settle it without a pool/obligation deadlock
|
||||
- **AND** after the drain commits, the failure recorder returns without changing attempts, errors or settlement
|
||||
|
||||
#### Scenario: A successful settlement waits for the pool before the obligation
|
||||
|
||||
- **WHEN** one transaction holds a pool row and another, holding the rendezvous, calls the settlement function with effects for an obligation of that pool
|
||||
- **THEN** the call waits for the pool without holding the obligation row
|
||||
- **AND** after the first transaction settles the obligation and commits, the call returns without writing an effect row or changing the obligation
|
||||
@@ -0,0 +1,37 @@
|
||||
Tests follow `docs/testing.md`: open through `dbtest`, build committed fixtures with the test kit or the package's existing fixtures, restrict reads to their ids, and pass twice without resetting between runs. The lock test uses `dbtest.AwaitLockWait`, never a sleep; transactions A and B each run `SET LOCAL ROLE console_runtime`. Helpers carry doc comments; table rows carry their expected values explicitly.
|
||||
|
||||
Sections 1–3 form one change; section 4 covers checks and landing. No existing golden, screen, route, test result or user-visible string is expected to change; any such change stops for review. Learn script usage from its header or source.
|
||||
|
||||
The break list is a file outside the repository. Use the format in `scripts/mutation-check/main.go`. Each break must compile (`go vet` its package while applied, then restore). Migration breaks are command records that reset the test databases and run the named test with `test/.env` loaded, exit 1 and the listed `want_new`. The scratch migration test migrates its own database and needs no shared reset. Every `want_new` below is printed only by a failing assertion. Run the list with `go run ./scripts/mutation-check -list <file> -side new` after a green control; all breaks must read CAUGHT, and their results belong in the report and commit message, not here.
|
||||
|
||||
## 1. Put the pool lock first
|
||||
|
||||
- [x] 1.1 Add the next core migration, provisionally `internal/db/migrations/00038_settlement_locks_pool_first.sql`. Up replaces migration 19's Up function with its pool lock immediately after the rendezvous assertion and before the obligation read. Change no other statement, default, error, privilege or write. Down restores migration 19's Up function exactly. Wrap each whole section in goose statement markers. Preserve owner, definer, search path and grants as described in the design.
|
||||
- [x] 1.2 Run `scripts/spdx-headers.sh <dir>` on directories with new SQL or Go files, and `make sqlc-generate`. Inspect any generated difference; no query signature changes are intended. Add no Go pool lock or new failure function.
|
||||
|
||||
## 2. Prove the order and preserve the writes
|
||||
|
||||
- [x] 2.1 Add `TestSettleObligationLocksPoolBeforeObligation` in `internal/entitlements/obligation_lock_order_test.go`. Build a carrying pool and rule with the package's fixtures, call `CommitRuleChangeTx` under `BeginRuleChange`, then commit the pending obligation without draining it. Open two materializing transactions. Table rows: `pending failure` and `pending success`; each carries its call kind and expected final status `settled`, attempts `1` and null error; the success row also carries zero late effects. A locks the pool. B calls `MarkObligationFailed` (budget 5) or `SettleObligation` without a Go pool prelock. B's success payload contains one valid effect on the fixture's key. Observe B's wait by backend PID, then A probes the obligation `FOR UPDATE NOWAIT` inside a savepoint. Failure text: `%s: obligation locked before pool: %v`. Roll back the probe savepoint, settle in A with an empty effect payload and commit, then join B. Verify both calls succeeded and the obligation stayed at A's result, including its settlement timestamp. On the success row, check the late effect count, which must be zero. Failure texts: `%s: settlement did not wait for the pool: %v` (early-return abort probe), `%s: settlement returned an error: %v`, `%s: late settlement changed the obligation: got %v, want %v`, `%s: late settlement wrote %d effects, want 0`. Use `t.Errorf` for the final state and effect checks so a broken settled guard reports both. Clean up both transactions and join the goroutine even if an assertion fails.
|
||||
- [x] 2.2 Add `TestSettlementPoolFirstMigrationUpDownUp` in `internal/db/settlement_pool_first_test.go`, using `dbtest.Scratch`, `UpTo` and `ApplyVersion`, following `displacement_holder_rank_test.go`. Capture `pg_get_functiondef` and owner/ACL before Up; check the definer, pinned search path, same signature/defaults, owner and execution rights after each step. Down must equal the captured old definition. Exercise a runtime success and failure on separate pending obligations under the shared rendezvous after Up and the second Up. Failure texts: `%s: settlement function privileges changed: %v`, `down did not restore the previous settlement function`, `%s: runtime settlement returned %v`; step labels are `first up`, `down` and `second up`. Use a fixture the test owns; no shared test database migration. Existing enclosure tests retain coverage of refusal without the rendezvous.
|
||||
- [x] 2.3 Run the new entitlement test twice without a reset between runs, after the required migration reset. Run the scratch migration test. Run the existing `TestRuleChangeEnclosure`, `TestSettleObligationAttemptBudget`, `TestDrainObligationAttemptBudgetAndDeadLetter`, `TestRuleChangeCommitRecordsEffects`, `TestRuleChangeBatchCommitRoutesEffectsByKey`, `TestRuleChangeBatchDeferredDrainsOncePerPool` and `TestRuleChangeCommitGolden`. Keep every golden unchanged. Run the changed test packages with `-race -p 1 -count=1`, one package at a time, because the new lock test uses goroutines.
|
||||
- [x] 2.4 Write these ten breaks in the external list. Each `old` is the exact contiguous text between the stated anchors, inclusive, in the new migration; verify that it occurs exactly once. Retain the anchors in `new` except for the intended edit.
|
||||
- **1. Restore the inversion.** Anchor `old` from Up's `PERFORM core.assert_rendezvous(FALSE);`, immediately followed by the pool lock, through the settled guard. Move that pool lock after the guard. `TestSettleObligationLocksPoolBeforeObligation/pending_failure`, `want_new`: `pending failure: obligation locked before pool`.
|
||||
- **2. Omit the pool lock.** Anchor `old` as `PERFORM core.assert_rendezvous(FALSE);` followed by the pool lock; only Up has that pair. Keep the assertion and remove the pool lock. Same test and row, `want_new`: `pending failure: settlement did not wait for the pool`.
|
||||
- **3. Lock only for a failure.** Use break 2's `old`; wrap its pool lock in `IF p_failure IS NOT NULL THEN ... END IF`. The success payload's pool foreign key then waits while holding the obligation. `TestSettleObligationLocksPoolBeforeObligation/pending_success`, `want_new`: `pending success: obligation locked before pool`.
|
||||
- **4. Write a late failure.** Anchor `old` as the settled guard followed by `IF p_failure IS NOT NULL THEN`; only Up has that adjacency. Remove the guard. The late failure violates the obligation's settled-shape constraint. `TestSettleObligationLocksPoolBeforeObligation/pending_failure`, `want_new`: `pending failure: settlement returned an error`.
|
||||
- **5. Write a late success.** Use break 4's same `old` and `new`, in a second record. `TestSettleObligationLocksPoolBeforeObligation/pending_success`, `want_new`: `pending success: late settlement wrote 1 effects, want 0`.
|
||||
- **6. Keep the new order in Down.** Anchor `old` from Down's `PERFORM core.assert_rendezvous(FALSE);` through its pool lock; only Down puts the obligation read and guard between them. Move the pool lock directly after the assertion. `TestSettlementPoolFirstMigrationUpDownUp`, `want_new`: `down did not restore the previous settlement function`.
|
||||
- **7. Lose Down's privileges.** Anchor `old` as the complete Down section from `-- +goose Down` through its final `-- +goose StatementEnd`. Replace its function replacement with `DROP FUNCTION core.settle_obligation(UUID, UUID, JSONB, TEXT, INTEGER);` and `CREATE FUNCTION` with the old body, without restoring owner or grants. Keep the goose markers. The migration user owns the new function and the old execution grant is lost. `TestSettlementPoolFirstMigrationUpDownUp`, `want_new`: `down: settlement function privileges changed`.
|
||||
- **8. Break the obligation read.** Anchor `old` from Up's assertion and immediately following pool lock through its obligation read's `FOR UPDATE;`. Misspell `o.attempts` as `o.attempt_count` in that read. `TestSettlementPoolFirstMigrationUpDownUp`, `want_new`: `first up: runtime settlement returned`.
|
||||
- **9. Change a settled obligation late.** Use break 4's `old` and `new` in a third record. `TestSettleObligationLocksPoolBeforeObligation/pending_success`, `want_new`: `pending success: late settlement changed the obligation`.
|
||||
- **10. Lose Up's privileges.** Anchor `old` from the header's closing rule line through Up's `CREATE OR REPLACE FUNCTION core.settle_obligation(`. Put `DROP FUNCTION core.settle_obligation(UUID, UUID, JSONB, TEXT, INTEGER);` before the function and make it `CREATE FUNCTION`. The new function carries PUBLIC execute, which migration 18 revoked. `TestSettlementPoolFirstMigrationUpDownUp`, `want_new`: `first up: settlement function privileges changed`.
|
||||
|
||||
## 3. Describe the resulting order
|
||||
|
||||
- [x] 3.1 Rewrite `docs/database-locks.md`'s rule-change Commit and Drain entries: shared rendezvous for a failure record, pool first inside `core.settle_obligation`, then each obligation in batch order; later function calls reuse the held pool. Delete the "Recording a failed drain locks the obligation row before the pool row" bullet under "Where the paths differ". Keep plan-act paths as they are and keep #186 separate.
|
||||
|
||||
## 4. Checks and landing
|
||||
|
||||
- [x] 4.1 Settle the migration's next free number and test constants at landing. Run `make test` with the isolated test stack up, and `openspec validate settlement-locks-pool-first --strict`. `make lint` must report `notebook-citations: ok`.
|
||||
- [x] 4.2 Run the external break list, require every break CAUGHT, restore the files and reset the test databases after migration breaks. No new captures or golden updates are needed. Record checks and break results in the report and commit message.
|
||||
- [x] 4.3 Land one commit for #196. The migration rewrites no data and needs no deployment-specific compatibility work. Downgrading restores the old function and the deadlock risk.
|
||||
Reference in New Issue
Block a user