test(multiplayer): cover concurrent proposal-expiry recovery race

Closes the 'concurrent proposal-recovery expiry races' gap noted in
§8.46. GetProposal (read-side recovery) and RespondToProposal both
run the identical expiry-advance SQL in their own transaction, so any
number of them can observe the same past-expiry proposal at once —
this had never been exercised concurrently, only sequentially (the
existing late-response test drives one call at a time).

TestPostgreSQLConcurrentProposalExpiryRecoveryAppliesCooldownsExactlyOnce
races 8 concurrent GetProposal/RespondToProposal calls, each with a
distinct 'now' past the proposal window, against one proposal and
asserts: EXPIRED lands on the proposal and both tickets exactly once,
a PROPOSAL_TIMEOUT penalty lands exactly once per offending player
(not once per racing transaction), and no idempotency row survives a
closed-proposal response. The design already defends against this —
ProposalParticipantExpireSQL only ever flips a still-PENDING row
once, so a losing racer's 'now' can't match
recordProposalTimeoutCooldowns' responded_at filter — this test is
what actually proves that holds under real concurrent load rather
than by inspection.

Verified: real postgres:17-alpine container, go test -tags
integration ./store/... -run
TestPostgreSQLConcurrentProposalExpiryRecoveryAppliesCooldownsExactlyOnce
-race -count=3 clean; full -tags integration ./store/... -race run
clean; full non-integration go build/vet/test -race clean across
every server package; container removed after the run.
This commit is contained in:
Josh Creek
2026-09-04 17:26:44 +01:00
parent ce17a45afb
commit f09ef7da8f
2 changed files with 108 additions and 1 deletions
+107
View File
@@ -1020,6 +1020,113 @@ func TestPostgreSQLLateProposalResponseCommitsExpiryRecovery(t *testing.T) {
}
}
// TestPostgreSQLConcurrentProposalExpiryRecoveryAppliesCooldownsExactlyOnce
// covers the race §8.46 flagged as still open: multiple concurrent recovery
// paths (a read-side GetProposal from each participant polling for an
// update, and a RespondToProposal arriving right at the same boundary) can
// all observe the same past-expiry proposal simultaneously. Every one of
// them runs the identical expiry-advance SQL in its own transaction, so this
// proves that racing recovery does not multiply the durable side effects: a
// PROPOSAL_TIMEOUT cooldown must land exactly once per offending player, not
// once per racing transaction that happened to perform the PENDING ->
// TIMED_OUT flip. The design's own defense is that ProposalParticipantExpireSQL
// only ever flips a still-PENDING row once, and recordProposalTimeoutCooldowns
// only cooldowns participants whose responded_at equals this transaction's
// own `now` -- so a loser transaction's `now` simply matches nothing.
func TestPostgreSQLConcurrentProposalExpiryRecoveryAppliesCooldownsExactlyOnce(t *testing.T) {
db := openIntegrationPostgres(t)
applyIntegrationMigrations(t, db)
now := time.Now().UTC().Truncate(time.Microsecond)
ctx := context.Background()
for _, player := range []string{"race-expiry-a", "race-expiry-b"} {
if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ($1, $1)`, player); err != nil {
t.Fatal(err)
}
}
for i, player := range []string{"race-expiry-a", "race-expiry-b"} {
if _, err := db.ExecContext(ctx, `INSERT INTO queue_tickets (ticket_id, player_id, playlist, state, client_build, protocol_version, enqueued_at, expires_at) VALUES ($1, $2, 'casual', 'QUEUED', 'integration-build', 1, $3, $4)`, fmt.Sprintf("race-expiry-ticket-%d", i), player, now, now.Add(time.Minute)); err != nil {
t.Fatal(err)
}
}
proposal, err := domain.NewProposal("race-expiry-proposal", domain.Casual, []string{"race-expiry-a", "race-expiry-b"}, now)
if err != nil {
t.Fatal(err)
}
if err := CreateProposal(ctx, db, proposal, map[string]string{"race-expiry-a": "race-expiry-ticket-0", "race-expiry-b": "race-expiry-ticket-1"}, now); err != nil {
t.Fatalf("create proposal: %v", err)
}
late := now.Add(domain.ProposalWindow + time.Second)
const racers = 8
var wg sync.WaitGroup
errs := make([]error, racers)
wg.Add(racers)
for i := 0; i < racers; i++ {
go func(i int) {
defer wg.Done()
// Each racer's `now` is distinct (and every one is past expiry), so a
// real implementation bug would show up as several of them believing
// they were the one that performed the PENDING -> TIMED_OUT flip.
racerNow := late.Add(time.Duration(i) * time.Millisecond)
switch i % 3 {
case 0:
_, errs[i] = GetProposal(ctx, db, "race-expiry-a", proposal.ProposalID, racerNow)
case 1:
_, errs[i] = GetProposal(ctx, db, "race-expiry-b", proposal.ProposalID, racerNow)
default:
_, errs[i] = RespondToProposal(ctx, db, "race-expiry-a", proposal.ProposalID, fmt.Sprintf("race-expiry-key-%04d", i), true, 0, racerNow)
}
}(i)
}
wg.Wait()
for i, err := range errs {
// GetProposal never errors on an already-expired proposal (it's a pure
// read-with-recovery); RespondToProposal on an already-closed proposal
// must report exactly ErrProposalClosed, nothing else.
if err != nil && !errors.Is(err, domain.ErrProposalClosed) {
t.Fatalf("racer %d: unexpected error %v", i, err)
}
}
var proposalState string
if err := db.QueryRow(`SELECT state FROM proposals WHERE proposal_id = 'race-expiry-proposal'`).Scan(&proposalState); err != nil {
t.Fatal(err)
}
if proposalState != "EXPIRED" {
t.Fatalf("proposal state = %s, want EXPIRED", proposalState)
}
var ticketA, ticketB string
if err := db.QueryRow(`SELECT state FROM queue_tickets WHERE ticket_id = 'race-expiry-ticket-0'`).Scan(&ticketA); err != nil {
t.Fatal(err)
}
if err := db.QueryRow(`SELECT state FROM queue_tickets WHERE ticket_id = 'race-expiry-ticket-1'`).Scan(&ticketB); err != nil {
t.Fatal(err)
}
if ticketA != "EXPIRED" || ticketB != "EXPIRED" {
t.Fatalf("tickets not expired exactly once: a=%s b=%s", ticketA, ticketB)
}
// The crux of the race: exactly one PROPOSAL_TIMEOUT penalty per player,
// however many transactions raced to observe the expiry.
var penaltiesA, penaltiesB int
if err := db.QueryRow(`SELECT count(*) FROM penalties WHERE kind = 'PROPOSAL_TIMEOUT' AND player_id = 'race-expiry-a'`).Scan(&penaltiesA); err != nil {
t.Fatal(err)
}
if err := db.QueryRow(`SELECT count(*) FROM penalties WHERE kind = 'PROPOSAL_TIMEOUT' AND player_id = 'race-expiry-b'`).Scan(&penaltiesB); err != nil {
t.Fatal(err)
}
if penaltiesA != 1 || penaltiesB != 1 {
t.Fatalf("cooldown was not applied exactly once per player: a=%d b=%d", penaltiesA, penaltiesB)
}
var idempotencyRows int
if err := db.QueryRow(`SELECT count(*) FROM idempotency_keys WHERE scope = $1`, ProposalResponseIdempotencyScope).Scan(&idempotencyRows); err != nil {
t.Fatal(err)
}
if idempotencyRows != 0 {
t.Fatalf("closed-proposal responses left stray idempotency rows: %d", idempotencyRows)
}
}
// TestPostgreSQLCancellingAProposedTicketImmediatelyRequeuesTheOtherParticipant
// covers the responsiveness gap the decline/timeout fixes above left bounded
// but not closed: cancelling a ticket that's part of an OPEN proposal used