mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-10 16:04:04 +00:00
fix(multiplayer): serve queue candidates when Redis is down, not just empty
Closes part of the 'live Redis failover' gap in §8.46, found by reproducing a genuine Redis outage (not just an empty/partial cache) against CandidateProjection.Snapshot with a killed miniredis instance. CandidateProjection.Snapshot funnelled two different situations into the same code path: the index erroring outright (Redis unreachable) and the index coming back empty (ambiguous — a genuinely empty queue, or a lost keyspace). Both went through Repair, which itself calls Index.Rebuild — a second Redis round-trip that fails for exactly the same reason the first one did. The result: a real Redis outage, or the window during a failover, made Snapshot fail outright even though PostgreSQL — the documented authoritative source everywhere (RedisCandidateIndex's own comment, cmd/matcher, cmd/control-plane's --redis-addr help text all call it a rebuildable/optional acceleration layer) — was completely healthy. Matchmaking would stop entirely on a Redis outage despite the architecture explicitly not requiring that. Snapshot now falls back to serving Source (PostgreSQL) directly whenever the index errors OR comes back empty, and only best-effort attempts to repopulate Redis afterward — that attempt's outcome is deliberately ignored, since a caller must never be denied service just because the opportunistic rebuild also hit the same down Redis. Snapshot still fails when Source itself is unavailable; the fallback is not unconditional. Verified: reproduced the bug first (killed-miniredis Snapshot call failed even though Source was healthy), then fixed it. go build/vet clean; all pre-existing store-package tests pass unmodified, including the two live-redis:7-alpine-container tests (TestRealRedisCandidateIndexUpsertSnapshotRemove, TestRealRedisCandidateProjectionRepairsAfterFlush, run against a real container and torn down after). Two new tests cover the fallback directly (killed miniredis, Source still served, exactly one Source call) and that the fallback is not unconditional (both Redis and Source down still fails). Full go test ./... -race clean across every server package. Remaining: live matcher-worker-under-load-during-failover integration, i.e. running the actual matcher process against a real Redis that goes down mid-run under concurrent load, not just this unit-level reproduction.
This commit is contained in:
+1
-1
@@ -1251,7 +1251,7 @@ The allocated-runtime result reporter now keeps a completed match in `RESULTS` u
|
||||
|---|---|---|
|
||||
| 8.44 `[D:8.3,8.4,8.28,8.31]` | **LOCAL COMPLETE; PRODUCTION GATE OPEN.** Go observability package encodes queue/proposal/match/server IDs and lifecycle stage while recursively redacting auth/relay tokens and credentials. `Service.Log` is wired to mutation and read routes at every outcome, and `cmd/control-plane` writes those events as JSON lines to stderr | `server/observability/` covers correlation fields, nested secret redaction, content-aware credential canaries and unnamed-event rejection; API tests cover lifecycle event wiring without logging error text. A production metrics/traces backend and dashboard/alert routing remain open; the local logger is intentionally stderr-only |
|
||||
| 8.45 `[D:8.2,8.44]` | **LOCAL COMPLETE; PRODUCTION GATE OPEN.** Go observability package turns the documented RTT, allocation/connect latency, result-success, API-latency and tick/headroom thresholds into executable window checks; the API exporter emits a bounded cumulative latency histogram suitable for querying the documented p95 API SLO | `server/observability/slo.go`, `metrics.go`, `deploy/observability/prometheus-rules.yaml` and adversarial tests cover healthy/violating/empty windows, fixed operation/status labels, cumulative bucket boundaries, arbitrary-path cardinality safety, and optional API p95/5xx alerts. Production scrape configuration, alert routing, wait/MMR/proposal/flood/cost series and runbooks remain |
|
||||
| 8.46 `[D:8.5,8.7,8.9,8.10,8.14,8.18,8.21,8.23,8.25]` | **IN PROGRESS.** Go unit/race coverage spans the current domain/store/supervisor policies, and fuzz targets now exercise queue input, result payload hashing and revision events. The local gate uses the pinned headless Godot container when the native executable is unavailable or crashes by signal, while preserving ordinary nonzero test failures, so its full cross-language suite remains runnable without an image export | `scripts/verify_multiplayer_local.sh` passed end to end on the current tree: Go normal/race/vet, all three bounded fuzz targets, 212 Godot tests, contracts, migrations, and manifests. `server/domain/*_test.go`, `server/store/*_test.go`, `server/supervisor/*_test.go`, `server/migrations/*_test.go` and `server/domain/fuzz_test.go` provide the underlying coverage; PostgreSQL live migration execution now runs clean (§8.5), and five real-concurrency cases are covered against a live database with `-race`: §8.14's queue-heartbeat revision race, §8.18's two-matcher contested-ticket race, §8.30's cross-allocator-replica capacity race, §8.21/§8.25's concurrent identical-result-submission race, and now `TestPostgreSQLConcurrentProposalExpiryRecoveryAppliesCooldownsExactlyOnce`, which races 8 concurrent `GetProposal`/`RespondToProposal` calls (mixed read-recovery and a late accept) against one already-expired proposal and proves the design's own defense holds: `ProposalParticipantExpireSQL` only ever flips a still-PENDING row once, so a losing racer's `now` never matches `recordProposalTimeoutCooldowns`' `responded_at = $2` filter and cannot double-apply a `PROPOSAL_TIMEOUT` penalty -- verified against a real PostgreSQL container, `-race`, 3 repeated runs plus a full store-package integration run, all clean; the "lost Redis" fixture is covered live against a real server (§8.14: real TTL expiry, repair-after-`FLUSHALL`; fake Steam/allocator fixtures are §8.47's testkit, already done). Live Redis failover mid-write under load remains |
|
||||
| 8.46 `[D:8.5,8.7,8.9,8.10,8.14,8.18,8.21,8.23,8.25]` | **IN PROGRESS.** Go unit/race coverage spans the current domain/store/supervisor policies, and fuzz targets now exercise queue input, result payload hashing and revision events. The local gate uses the pinned headless Godot container when the native executable is unavailable or crashes by signal, while preserving ordinary nonzero test failures, so its full cross-language suite remains runnable without an image export | `scripts/verify_multiplayer_local.sh` passed end to end on the current tree: Go normal/race/vet, all three bounded fuzz targets, 212 Godot tests, contracts, migrations, and manifests. `server/domain/*_test.go`, `server/store/*_test.go`, `server/supervisor/*_test.go`, `server/migrations/*_test.go` and `server/domain/fuzz_test.go` provide the underlying coverage; PostgreSQL live migration execution now runs clean (§8.5), and five real-concurrency cases are covered against a live database with `-race`: §8.14's queue-heartbeat revision race, §8.18's two-matcher contested-ticket race, §8.30's cross-allocator-replica capacity race, §8.21/§8.25's concurrent identical-result-submission race, and now `TestPostgreSQLConcurrentProposalExpiryRecoveryAppliesCooldownsExactlyOnce`, which races 8 concurrent `GetProposal`/`RespondToProposal` calls (mixed read-recovery and a late accept) against one already-expired proposal and proves the design's own defense holds: `ProposalParticipantExpireSQL` only ever flips a still-PENDING row once, so a losing racer's `now` never matches `recordProposalTimeoutCooldowns`' `responded_at = $2` filter and cannot double-apply a `PROPOSAL_TIMEOUT` penalty -- verified against a real PostgreSQL container, `-race`, 3 repeated runs plus a full store-package integration run, all clean; the "lost Redis" fixture is covered live against a real server (§8.14: real TTL expiry, repair-after-`FLUSHALL`; fake Steam/allocator fixtures are §8.47's testkit, already done). **Fixed a real Redis-failover bug found while chasing this gap**: `CandidateProjection.Snapshot` funnelled "the index errored" (Redis unreachable) and "the index came back empty" (ambiguous: truly empty, or a lost keyspace) into the same `Repair` path -- but `Repair` itself calls `Index.Rebuild`, a second Redis round-trip that fails for exactly the same reason the first one did. A genuine Redis outage or mid-failover window therefore made `Snapshot` fail outright even though PostgreSQL, the documented authoritative source, was completely healthy -- contradicting Redis's own documented status everywhere (`RedisCandidateIndex`'s comment, `cmd/matcher`, `cmd/control-plane`'s `--redis-addr` help text) as an optional, rebuildable acceleration layer. `Snapshot` now falls back to serving `Source` directly whenever the index errors or comes back empty, attempting to repopulate Redis only best-effort (its outcome is deliberately ignored) — verified with both a killed miniredis instance and a real `redis:7-alpine` container (existing `TestRealRedisCandidateIndexUpsertSnapshotRemove`/`TestRealRedisCandidateProjectionRepairsAfterFlush` still pass unmodified). Live matcher-worker-under-load-during-failover integration remains |
|
||||
| 8.47 `[D:8.7,8.30]` | **IN PROGRESS.** Offline testkit provides deterministic fake Steam verification and fake allocation with forced failure injection; the independent Compose runner drives fake-Steam session issuance, real HTTP queue create/heartbeat/cancel, matcher-backed six-player proposal formation/acceptance, and idempotency-conflict checks | `server/testkit/` covers verified identity/replay, unknown identity, wrong App ID, expiry, no capacity, compatibility-key conflict, idempotent allocation replay and cloud-free forced allocation failure; the Compose API/matcher slice is wired into CI, while live exhaustive matrix and production Steam remain |
|
||||
| 8.48 `[D:8.10,8.14,8.17,8.18,8.27,8.31,8.35,8.47]` | **LOCAL COMPLETE; PRODUCTION GATE OPEN.** Offline testkit exercises verified queue projection → ranked six-player proposal → ENet allocation → assignment-ready manifest → certified durable result receipt; `compose.allocated-smoke.yml` independently runs the real testkit API, matcher, allocator, maintenance, HTTPS Agones-shaped provider, PostgreSQL, and game-server supervisor with generated TLS, roster, and signed workload credentials. It verifies an expired ranked reconnect becomes one durable abandonment/cooldown, queue/proposal/allocation binding, authenticated result, idempotent retry, shutdown acknowledgment, durable receipt/audit rows, and supervised game-process stop without repurposing the Phase 6 fixture | `scripts/verify_allocated_compose.sh` passed on 2026-09-04 in this workspace; `.github/workflows/allocated-compose.yml` runs `make verify-allocated-compose`. Real Agones/kind and production evidence remain open |
|
||||
| 8.49 `[D:8.25,8.26,8.28,8.29,8.30,8.31,8.35,8.36]` | **IN PROGRESS.** `scripts/verify_kind_agones.sh` creates a disposable kind cluster, installs pinned Agones, loads the real `game-server` image, applies the Fleet in an explicitly separate Agones-only supervisor/UDP readiness mode, and verifies readiness plus allocation of a dynamic UDP endpoint; `.github/workflows/agones-integration.yml` runs it for infrastructure changes and on demand | The cloud-free runner is committed and fails clearly when Docker/kind/Helm are unavailable. CI/live evidence for production control-plane registration, roster/no-show, both readiness stages, races, multi-match node, result-pending reconciliation, drain, and rollback remains open |
|
||||
|
||||
@@ -47,6 +47,70 @@ func TestCandidateProjectionDoesNotReturnCacheWhenRepairSourceFails(t *testing.T
|
||||
}
|
||||
}
|
||||
|
||||
// TestCandidateProjectionFallsBackToSourceWhenRedisIsEntirelyUnreachable
|
||||
// covers the gap multiplayer-next.md §8.46 named "live Redis failover":
|
||||
// Redis is documented everywhere (RedisCandidateIndex's own comment,
|
||||
// cmd/matcher, cmd/control-plane) as an optional, rebuildable acceleration
|
||||
// layer over PostgreSQL authority. Before this fix, Snapshot funnelled a
|
||||
// genuine Redis connection failure into the same Repair path as an empty
|
||||
// cache -- but Repair's own Index.Rebuild call also needs Redis, so it failed
|
||||
// for the identical reason, and Snapshot returned an error even though the
|
||||
// authoritative Source was perfectly healthy. A real Redis outage or
|
||||
// mid-failover window would have taken matchmaking down completely.
|
||||
func TestCandidateProjectionFallsBackToSourceWhenRedisIsEntirelyUnreachable(t *testing.T) {
|
||||
mini, err := miniredis.Run()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
client := redis.NewClient(&redis.Options{Addr: mini.Addr()})
|
||||
defer client.Close()
|
||||
mini.Close() // Redis is now entirely unreachable, not merely empty or stale.
|
||||
|
||||
now := time.Unix(1000, 0).UTC()
|
||||
candidate := domain.Candidate{TicketID: "down-ticket", PlayerID: "down-player", EnqueuedAt: now}
|
||||
sourceCalls := 0
|
||||
projection := CandidateProjection{
|
||||
Index: RedisCandidateIndex{Client: client, Prefix: "down", TTL: time.Minute},
|
||||
Source: func(context.Context, time.Time) ([]domain.Candidate, error) {
|
||||
sourceCalls++
|
||||
return []domain.Candidate{candidate}, nil
|
||||
},
|
||||
}
|
||||
got, err := projection.Snapshot(context.Background(), now)
|
||||
if err != nil {
|
||||
t.Fatalf("Snapshot failed while Redis was down, even though Source (PostgreSQL) was healthy: %v", err)
|
||||
}
|
||||
if len(got) != 1 || got[0].TicketID != candidate.TicketID {
|
||||
t.Fatalf("fallback snapshot = %+v, want the durable candidate served directly", got)
|
||||
}
|
||||
if sourceCalls != 1 {
|
||||
t.Fatalf("Source calls = %d, want exactly 1", sourceCalls)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCandidateProjectionStillFailsWhenBothRedisAndSourceAreDown proves the
|
||||
// fallback isn't unconditional: if PostgreSQL itself is also unavailable,
|
||||
// Snapshot must still fail rather than silently return an empty match pool.
|
||||
func TestCandidateProjectionStillFailsWhenBothRedisAndSourceAreDown(t *testing.T) {
|
||||
mini, err := miniredis.Run()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
client := redis.NewClient(&redis.Options{Addr: mini.Addr()})
|
||||
defer client.Close()
|
||||
mini.Close()
|
||||
|
||||
projection := CandidateProjection{
|
||||
Index: RedisCandidateIndex{Client: client, Prefix: "down", TTL: time.Minute},
|
||||
Source: func(context.Context, time.Time) ([]domain.Candidate, error) {
|
||||
return nil, context.DeadlineExceeded
|
||||
},
|
||||
}
|
||||
if _, err := projection.Snapshot(context.Background(), time.Unix(1000, 0)); err == nil {
|
||||
t.Fatal("Snapshot succeeded with both Redis and the durable source unavailable")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCandidateProjectionRepairsEmptyIndexFromDurableSource(t *testing.T) {
|
||||
mini, err := miniredis.Run()
|
||||
if err != nil {
|
||||
|
||||
@@ -43,27 +43,38 @@ func (p CandidateProjection) Repair(ctx context.Context, now time.Time) error {
|
||||
return p.Index.Rebuild(ctx, candidates)
|
||||
}
|
||||
|
||||
// Snapshot never fails just because Redis specifically is unreachable.
|
||||
// RedisCandidateIndex is documented everywhere (this type's own comment,
|
||||
// cmd/matcher, cmd/control-plane's --redis-addr help text) as an optional,
|
||||
// rebuildable acceleration layer over PostgreSQL authority -- but until this
|
||||
// fix, a genuine Redis outage (not merely an empty or partial cache, an
|
||||
// actual connection failure) made Snapshot fail outright: the old code
|
||||
// treated "the index errored" and "the index came back empty" identically,
|
||||
// funnelling both into Repair, which itself calls Index.Rebuild -- a second
|
||||
// Redis round-trip that fails for exactly the same reason the first one did.
|
||||
// A Redis failover or restart would have taken matchmaking down completely
|
||||
// even though the authoritative Source (PostgreSQL) was perfectly healthy.
|
||||
// Now: an index error or an empty read both fall back to serving Source
|
||||
// directly, and only attempt to repopulate Redis on a best-effort basis --
|
||||
// its outcome is deliberately ignored, since a caller must never be denied
|
||||
// service just because the rebuild's own Redis write also failed.
|
||||
func (p CandidateProjection) Snapshot(ctx context.Context, now time.Time) ([]domain.Candidate, error) {
|
||||
if p.Source == nil {
|
||||
return nil, fmt.Errorf("invalid candidate repair source")
|
||||
}
|
||||
candidates, err := p.Index.Snapshot(ctx, now)
|
||||
if err == nil {
|
||||
// An empty index is indistinguishable from a Redis restart or a lost
|
||||
// keyspace. Rebuild from PostgreSQL before returning so queued players
|
||||
// are not hidden until the next enqueue mutation.
|
||||
if len(candidates) == 0 {
|
||||
if err := p.Repair(ctx, now); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return p.Index.Snapshot(ctx, now)
|
||||
}
|
||||
if err == nil && len(candidates) > 0 {
|
||||
return candidates, nil
|
||||
}
|
||||
if err := p.Repair(ctx, now); err != nil {
|
||||
return nil, err
|
||||
// Either the index errored outright, or came back empty -- indistinguishable
|
||||
// from a Redis restart or a lost keyspace. Consult PostgreSQL, the
|
||||
// authoritative source, either way.
|
||||
source, sourceErr := p.Source(ctx, now)
|
||||
if sourceErr != nil {
|
||||
return nil, sourceErr
|
||||
}
|
||||
return p.Index.Snapshot(ctx, now)
|
||||
_ = p.Index.Rebuild(ctx, source)
|
||||
return source, nil
|
||||
}
|
||||
|
||||
func (r RedisCandidateIndex) keys() (string, string) {
|
||||
|
||||
Reference in New Issue
Block a user