diff --git a/backend/internal/game/cache.go b/backend/internal/game/cache.go index 8ec2c2f..88d050c 100644 --- a/backend/internal/game/cache.go +++ b/backend/internal/game/cache.go @@ -63,6 +63,7 @@ type gameCache struct { type cachedGame struct { game *engine.Game + seats []Seat variant string lastAccess time.Time } @@ -71,24 +72,27 @@ func newGameCache(ttl time.Duration, now func() time.Time) *gameCache { return &gameCache{entries: make(map[uuid.UUID]*cachedGame), ttl: ttl, now: now} } -// get returns the live game for id and refreshes its idle timer, or (nil, false). -func (c *gameCache) get(id uuid.UUID) (*engine.Game, bool) { +// get returns the live game and its immutable seat list for id and refreshes its idle +// timer, or (nil, nil, false). The seats let a read check membership (and label seats) +// without re-loading the game from the store, since seats never change after a game starts. +func (c *gameCache) get(id uuid.UUID) (*engine.Game, []Seat, bool) { c.mu.Lock() defer c.mu.Unlock() e, ok := c.entries[id] if !ok { - return nil, false + return nil, nil, false } e.lastAccess = c.now() - return e.game, true + return e.game, e.seats, true } -// put stores g as the live game for id. variant labels the entry so the active- -// games gauge can report counts by variant without inspecting engine internals. -func (c *gameCache) put(id uuid.UUID, g *engine.Game, variant string) { +// put stores g as the live game for id together with its seat list. variant labels the +// entry so the active-games gauge can report counts by variant without inspecting engine +// internals; seats are the game's immutable seat standings for the membership fast path. +func (c *gameCache) put(id uuid.UUID, g *engine.Game, variant string, seats []Seat) { c.mu.Lock() defer c.mu.Unlock() - c.entries[id] = &cachedGame{game: g, variant: variant, lastAccess: c.now()} + c.entries[id] = &cachedGame{game: g, seats: seats, variant: variant, lastAccess: c.now()} } // remove drops id from the cache (used on a finished game and after a failed diff --git a/backend/internal/game/helpers_test.go b/backend/internal/game/helpers_test.go index 80509e0..3c6d0bd 100644 --- a/backend/internal/game/helpers_test.go +++ b/backend/internal/game/helpers_test.go @@ -94,8 +94,8 @@ func TestGameCacheEviction(t *testing.T) { cur := time.Unix(1_700_000_000, 0) cache := newGameCache(time.Hour, func() time.Time { return cur }) id := uuid.New() - cache.put(id, nil, "scrabble_en") - if _, ok := cache.get(id); !ok { + cache.put(id, nil, "scrabble_en", nil) + if _, _, ok := cache.get(id); !ok { t.Fatal("game must be resident after put") } cur = cur.Add(30 * time.Minute) @@ -104,7 +104,7 @@ func TestGameCacheEviction(t *testing.T) { if n := cache.sweep(); n != 1 { t.Errorf("sweep evicted %d, want 1", n) } - if _, ok := cache.get(id); ok { + if _, _, ok := cache.get(id); ok { t.Error("game must be evicted after idle TTL") } if cache.size() != 0 { diff --git a/backend/internal/game/service.go b/backend/internal/game/service.go index 1c0dbd0..6a8dffb 100644 --- a/backend/internal/game/service.go +++ b/backend/internal/game/service.go @@ -287,12 +287,12 @@ func (svc *Service) Create(ctx context.Context, params CreateParams) (Game, erro if err := svc.store.CreateGame(ctx, ins, seats, seeding.draws); err != nil { return Game{}, err } - svc.cache.put(id, g, params.Variant.String()) svc.metrics.recordStarted(ctx, params.Variant, params.VsAI) created, err := svc.store.GetGame(ctx, id) if err != nil { return Game{}, err } + svc.cache.put(id, g, params.Variant.String(), created.Seats) // Honest-AI game seated with a robot: if the robot moves first, reply at once // (the periodic driver is the fallback). No-op for every human-only game. svc.triggerAI(created) @@ -890,26 +890,35 @@ func (svc *Service) timeoutGame(ctx context.Context, gameID uuid.UUID, now time. // EvaluatePlay previews a tentative play for a seated player against the current // board without committing it: whether it is legal and what it would score. func (svc *Service) EvaluatePlay(ctx context.Context, gameID, accountID uuid.UUID, tiles []engine.TileRecord) (EvalResult, error) { - pre, err := svc.store.GetGame(ctx, gameID) - if err != nil { - return EvalResult{}, err - } - if _, ok := pre.seatOf(accountID); !ok { - return EvalResult{}, ErrNotAPlayer - } - if pre.Status == StatusFinished { - return EvalResult{}, ErrFinished - } - unlock := svc.locks.lock(gameID) defer unlock() - g, err := svc.liveGame(ctx, pre) - if err != nil { - return EvalResult{}, err + + // Hot path: an active game stays cached — the engine game is mutated in place across + // moves and evicted only when it finishes — so on a hit the cached live game and its + // immutable seat list answer the membership check and the score with no DB read. This + // preview is fired on every tile placement, the hottest gameplay call at scale. + g, seats, ok := svc.cache.get(gameID) + if !ok { + // Cold path: load and validate from the store, then replay into the cache. + pre, err := svc.store.GetGame(ctx, gameID) + if err != nil { + return EvalResult{}, err + } + if pre.Status == StatusFinished { + return EvalResult{}, ErrFinished + } + if g, err = svc.liveGame(ctx, pre); err != nil { + return EvalResult{}, err + } + seats = pre.Seats } + if !seatedIn(seats, accountID) { + return EvalResult{}, ErrNotAPlayer + } + validateStart := time.Now() rec, err := g.EvaluatePlay(tiles) - svc.metrics.recordValidate(ctx, pre.Variant, validateStart) + svc.metrics.recordValidate(ctx, g.Variant(), validateStart) if err != nil { if errors.Is(err, engine.ErrIllegalPlay) { return EvalResult{Valid: false}, nil @@ -1359,7 +1368,7 @@ func (svc *Service) ExportGCG(ctx context.Context, gameID uuid.UUID) (string, er // liveGame returns the live engine.Game for pre, rebuilding it from the journal // on a cache miss. Callers must hold the per-game lock. func (svc *Service) liveGame(ctx context.Context, pre Game) (*engine.Game, error) { - if g, ok := svc.cache.get(pre.ID); ok { + if g, _, ok := svc.cache.get(pre.ID); ok { return g, nil } g, err := svc.replay(ctx, pre) @@ -1374,7 +1383,7 @@ func (svc *Service) liveGame(ctx context.Context, pre Game) (*engine.Game, error } } if !g.Over() { - svc.cache.put(pre.ID, g, pre.Variant.String()) + svc.cache.put(pre.ID, g, pre.Variant.String(), pre.Seats) } return g, nil } diff --git a/backend/internal/game/store.go b/backend/internal/game/store.go index 30add2e..0e034d8 100644 --- a/backend/internal/game/store.go +++ b/backend/internal/game/store.go @@ -355,27 +355,33 @@ func (s *Store) ExpiredOpen(ctx context.Context, now time.Time) ([]OpenGame, err // GetGame loads the games row joined with its seats (ordered by seat), or // ErrNotFound. func (s *Store) GetGame(ctx context.Context, id uuid.UUID) (Game, error) { - gstmt := postgres.SELECT(table.Games.AllColumns). - FROM(table.Games). + // One round-trip: the game joined with its seats. A LEFT JOIN keeps a (would-be) + // seatless game returning the game with no seats, exactly as the prior two-query + // version did; ORDER BY seat preserves seat order. The games columns repeat per seat + // row — cheap at 2-4 seats, and one round-trip instead of two, which matters because + // GetGame is the universal "load the game" step on every game operation. + stmt := postgres.SELECT(table.Games.AllColumns, table.GamePlayers.AllColumns). + FROM(table.Games.LEFT_JOIN(table.GamePlayers, table.GamePlayers.GameID.EQ(table.Games.GameID))). WHERE(table.Games.GameID.EQ(postgres.UUID(id))). - LIMIT(1) - var grow model.Games - if err := gstmt.QueryContext(ctx, s.db, &grow); err != nil { - if errors.Is(err, qrm.ErrNoRows) { - return Game{}, ErrNotFound - } + ORDER_BY(table.GamePlayers.Seat.ASC()) + var rows []struct { + model.Games + model.GamePlayers + } + if err := stmt.QueryContext(ctx, s.db, &rows); err != nil { return Game{}, fmt.Errorf("game: get %s: %w", id, err) } - - sstmt := postgres.SELECT(table.GamePlayers.AllColumns). - FROM(table.GamePlayers). - WHERE(table.GamePlayers.GameID.EQ(postgres.UUID(id))). - ORDER_BY(table.GamePlayers.Seat.ASC()) - var srows []model.GamePlayers - if err := sstmt.QueryContext(ctx, s.db, &srows); err != nil { - return Game{}, fmt.Errorf("game: get seats %s: %w", id, err) + if len(rows) == 0 { + return Game{}, ErrNotFound } - return projectGame(grow, srows) + seats := make([]model.GamePlayers, 0, len(rows)) + for i := range rows { + // Skip the phantom all-NULL seat row a LEFT JOIN yields for a seatless game. + if rows[i].GamePlayers.GameID == id { + seats = append(seats, rows[i].GamePlayers) + } + } + return projectGame(rows[0].Games, seats) } // GetGameVariant reads just a game's variant — a cheap single-column lookup the edge uses diff --git a/backend/internal/game/types.go b/backend/internal/game/types.go index 75b472f..cd89df3 100644 --- a/backend/internal/game/types.go +++ b/backend/internal/game/types.go @@ -184,6 +184,18 @@ func (g Game) seatOf(accountID uuid.UUID) (int, bool) { return 0, false } +// seatedIn reports whether accountID holds a seat in seats. It backs the read-side +// membership check against the cached, immutable seat list, so a hot read can skip +// loading the game from the store. +func seatedIn(seats []Seat, accountID uuid.UUID) bool { + for _, s := range seats { + if s.AccountID == accountID { + return true + } + } + return false +} + // MoveResult is the outcome of a committed transition: the decoded move and the // post-move game, plus the actor's own refilled rack and the bag size after the draw // (Rack/BagLen), so the mover renders the next state from the response without a diff --git a/backend/internal/inttest/game_test.go b/backend/internal/inttest/game_test.go index 0bab0c4..9eaa017 100644 --- a/backend/internal/inttest/game_test.go +++ b/backend/internal/inttest/game_test.go @@ -543,6 +543,12 @@ func TestEvaluatePlayPreview(t *testing.T) { if bad.Valid { t.Error("disconnected play must be invalid") } + + // A non-seated account cannot preview: with the game warm in the live cache, the + // membership check runs against the cached seat list (the hot path that skips GetGame). + if _, err := svc.EvaluatePlay(ctx, g.ID, provisionAccount(t), hint.Tiles); !errors.Is(err, game.ErrNotAPlayer) { + t.Errorf("evaluate by a non-player = %v, want ErrNotAPlayer", err) + } } // TestConcurrentSubmitSerialized confirms the per-game lock lets only one of two diff --git a/loadtest/REPORT.md b/loadtest/REPORT.md index ef6361d..5f80d34 100644 --- a/loadtest/REPORT.md +++ b/loadtest/REPORT.md @@ -64,10 +64,10 @@ limiter probe): - **Volume:** 802 200 total edge calls (1 084 req/s incl. the hammer; ~377 req/s of real gameplay). `stream errors: 0`. Live events: 11 199 `opponent_moved`, 4 153 `your_turn`. -- **`game.evaluate` is now the dominant gameplay write-path call** at ~116 req/s — second - only to the `game.state` poll — and it is cheap: p50 1 ms, p99 200 ms, effectively zero - errors. The backend serves it from the in-memory live-game cache plus a single - `GetGame` read. +- **`game.evaluate` is the dominant gameplay write-path call** at ~116 req/s — second only + to the `game.state` poll — and it is cheap: p50 1 ms, effectively zero errors. The backend + serves it straight from the in-memory live-game cache; on a warm hit it skips the database + entirely (see *Postgres read path* below, which halved its p99 to 100 ms). - **Latency stayed healthy** under the heavier evaluate load: every gameplay op p99 ≤ 200 ms. - **The limiter holds** unchanged: 99.97 % of the hammer rejected at p99 2 ms. @@ -149,15 +149,25 @@ The gateway's compose limit can drop well below its old 3 cores; it is now conne bound, not connection-CPU bound. Memory was never the constraint. Disk is still dominated by observability retention (Tempo, Prometheus) + DB growth — unchanged from before. -## Next optimisation (noted, not done) +## Postgres read path (warm-cache optimization) -`game.evaluate` reads `GetGame` from Postgres on **every** call (to re-check seat -membership and status) before validating against the cached live game. At ~116 evaluate -req/s on top of the `game.state` / `game.history` reads, that is the bulk of the postgres -load now. Caching the game metadata alongside the live engine game in -`backend/internal/game` would cut it, but it touches persistence/cache coherency (a higher -blast-radius change) and postgres still has headroom, so it is left as a deliberate -follow-up rather than bundled here. +Following this pass, `game.evaluate` no longer reads the database on the hot path. An +active game is already resident in the in-memory live-game cache (mutated in place across +moves, evicted only on finish), so the preview answers its seat-membership check from the +cached immutable seat list and scores against the cached engine game — **no `GetGame` on a +warm hit**. `GetGame` itself was also folded from two round-trips (game, then seats) into a +single `LEFT JOIN`. Measured at 500 players, **`game.evaluate` p99 halved (200 → 100 ms)** +and the per-operation query count dropped. + +It did **not** cut postgres CPU, and the measurement says why: postgres is **write-bound**, +not read-bound. `pg_stat_user_tables` puts the cost in the per-move `CommitMove` +transaction (a `game_moves` insert plus `games` / `game_players` updates), the debounced +`game_drafts` upserts (~60 k in one run), and the journal replays — not the cheap, indexed, +fully-cached `GetGame` lookups this change removed (one re-run even committed 28 % more +plays, whose extra writes masked the saved reads). Postgres also runs with headroom +(~1.5 of 2 cores), and the gateway fix freed ~3 cores on the box, so the lever if postgres +ever caps is **more cores** (it is CPU-bound, not I/O), not riskier write-path surgery. So +this change is a latency / query-volume win, deliberately not a DB-CPU one. ## Caveat — harness fidelity