From 3151e54ab3a34b4551feaab359a4a39ab66aaec7 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 21:32:34 +0100 Subject: [PATCH] fix(multiplayer): use locked rating for season rollover --- multiplayer-next.md | 2 ++ server/store/postgres_integration_test.go | 14 ++++++------- server/store/season_sql.go | 25 +++++++++++++++-------- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/multiplayer-next.md b/multiplayer-next.md index b576f956..4f37f1d5 100644 --- a/multiplayer-next.md +++ b/multiplayer-next.md @@ -1453,6 +1453,8 @@ The allocator’s durable bind now increments the match revision and writes a pa Ranked maintenance now marks expired seasons with no ranked profiles as rolled over, preventing an empty season from being selected and reconsidered on every maintenance pass; the boundary is covered by the integration-tag regression suite. +Season rollover now computes from the row locked inside its serializable transaction rather than a stale caller snapshot; the PostgreSQL integration regression deliberately passes a 1900 profile against a durable 2000 rating and verifies the 1875 result is preserved. + Ranked proposal admission no longer trusts the matcher’s `--ranked-random-arena` boolean. The Go domain now owns a named allowlist for the three floor-goal `ArenaRegistry` entries, and rejects unknown and elevated IDs before any proposal is created. The arena hand-off is now durable: the matcher deterministically selects an eligible floor-goal arena from the proposal ID, migration 0008 stores that path on proposals and matches and enforces it for new direct SQL writes, migration 0009 retains it on provider allocations, domain/store/provider boundaries and recovery lookups recheck the same allowlist, allocation claims and idempotency digests retain it, Agones applies it as a match-scoped annotation, and the supervisor overlays the allocated child’s `--arena-path`. Godot accepts only the same floor-goal `ArenaRegistry` paths and requires one for allocated ranked matches, so a stale Fleet default, an elevated variant, or an altered retry cannot substitute a ranked arena. The recovery worker also rejects a provider-recovered allocation whose arena differs from the durable request before recording or binding it. diff --git a/server/store/postgres_integration_test.go b/server/store/postgres_integration_test.go index ab40db48..462cca69 100644 --- a/server/store/postgres_integration_test.go +++ b/server/store/postgres_integration_test.go @@ -1218,7 +1218,7 @@ func TestPostgreSQLRankedSeasonRolloverIsExactlyOnce(t *testing.T) { if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ('season-player', 'season-steam')`); err != nil { t.Fatal(err) } - if _, err := db.ExecContext(ctx, `INSERT INTO ratings (player_id, rating, deviation, volatility, ranked_games) VALUES ('season-player', 1900, 100, 0.12, 25)`); err != nil { + if _, err := db.ExecContext(ctx, `INSERT INTO ratings (player_id, rating, deviation, volatility, ranked_games) VALUES ('season-player', 2000, 100, 0.12, 25)`); err != nil { t.Fatal(err) } if _, err := db.ExecContext(ctx, `INSERT INTO seasons (season_id, playlist, starts_at, ends_at) VALUES ('season-1', 'ranked', $1, $2)`, now.Add(-12*7*24*time.Hour), now); err != nil { @@ -1229,7 +1229,7 @@ func TestPostgreSQLRankedSeasonRolloverIsExactlyOnce(t *testing.T) { if err != nil || !applied { t.Fatalf("first season rollover = %+v applied=%v err=%v", updated, applied, err) } - if updated.Value != 1800 || updated.RD != 200 { + if updated.Value != 1875 || updated.RD != 200 { t.Fatalf("unexpected rolled rating: %+v", updated) } var rating float64 @@ -1240,17 +1240,17 @@ func TestPostgreSQLRankedSeasonRolloverIsExactlyOnce(t *testing.T) { if err := db.QueryRow(`SELECT count(*) FROM ranked_season_rollovers WHERE player_id = 'season-player' AND season_id = 'season-1'`).Scan(&markers); err != nil { t.Fatal(err) } - if rating != 1800 || markers != 1 { + if rating != 1875 || markers != 1 { t.Fatalf("durable rollover state rating=%v markers=%d", rating, markers) } - _, applied, err = ApplyRankedSeasonRollover(ctx, db, "season-player", "season-1", profile, now.Add(time.Second)) - if err != nil || applied { - t.Fatalf("duplicate season rollover applied=%v err=%v", applied, err) + duplicate, applied, err := ApplyRankedSeasonRollover(ctx, db, "season-player", "season-1", profile, now.Add(time.Second)) + if err != nil || applied || duplicate.Value != 1875 { + t.Fatalf("duplicate rollover = %+v applied=%v err=%v", duplicate, applied, err) } if err := db.QueryRow(`SELECT rating FROM ratings WHERE player_id = 'season-player'`).Scan(&rating); err != nil { t.Fatal(err) } - if rating != 1800 { + if rating != 1875 { t.Fatalf("duplicate rollover changed rating to %v", rating) } } diff --git a/server/store/season_sql.go b/server/store/season_sql.go index 21c6fd3f..536bc8f7 100644 --- a/server/store/season_sql.go +++ b/server/store/season_sql.go @@ -28,16 +28,24 @@ WHERE player_id = $1` // retry after a worker failure cannot apply compression twice or leave a // marker without its corresponding rating snapshot. func ApplyRankedSeasonRollover(ctx context.Context, db *sql.DB, playerID, seasonID string, profile domain.RankedProfile, now time.Time) (domain.RankedProfile, bool, error) { - if playerID == "" { - return domain.RankedProfile{}, false, fmt.Errorf("player ID is required") - } - updated, _, err := domain.ApplySeasonRollover(profile, seasonID) - if err != nil { - return domain.RankedProfile{}, false, err + if db == nil || playerID == "" || seasonID == "" || now.IsZero() { + return domain.RankedProfile{}, false, fmt.Errorf("invalid season rollover arguments") } + // The caller's profile is only a validation-compatible hint. The durable + // row is authoritative because a result update may have committed after the + // caller read its snapshot but before this transaction acquired the lock. + _ = profile + var updated domain.RankedProfile applied := false - err = RunSerializable(ctx, db, DefaultSerializableAttempts, func(ctx context.Context, tx *sql.Tx) error { - if _, err := tx.ExecContext(ctx, SeasonRatingLockSQL, playerID); err != nil { + err := RunSerializable(ctx, db, DefaultSerializableAttempts, func(ctx context.Context, tx *sql.Tx) error { + var locked domain.RankedProfile + var revision int64 + if err := tx.QueryRowContext(ctx, SeasonRatingLockSQL, playerID).Scan(&locked.Value, &locked.RD, &locked.Volatility, &locked.RankedGames, &revision); err != nil { + return err + } + var err error + updated, _, err = domain.ApplySeasonRollover(locked, seasonID) + if err != nil { return err } result, err := tx.ExecContext(ctx, SeasonRolloverInsertSQL, playerID, seasonID, updated.Value, updated.RD, updated.Volatility, updated.RankedGames, now) @@ -49,6 +57,7 @@ func ApplyRankedSeasonRollover(ctx context.Context, db *sql.DB, playerID, season return err } if changed == 0 { + updated = locked return nil } if _, err := tx.ExecContext(ctx, SeasonRatingUpdateSQL, playerID, updated.Value, updated.RD, updated.Volatility, now); err != nil {