From 25fdc2c2c8eb0137b77b6ecd6bcf9681f4f56580 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 10:40:10 +0100 Subject: [PATCH] fix(multiplayer): recover recorded allocations --- multiplayer-next.md | 7 +++--- multiplayer-todo.md | 2 +- server/allocator/worker.go | 14 +++++++++--- server/allocator/worker_test.go | 28 +++++++++++++++++++----- server/store/allocation_match_adapter.go | 4 ++++ server/store/allocation_match_sql.go | 25 +++++++++++++++++++++ 6 files changed, 68 insertions(+), 12 deletions(-) diff --git a/multiplayer-next.md b/multiplayer-next.md index 4bf31422..ca129456 100644 --- a/multiplayer-next.md +++ b/multiplayer-next.md @@ -67,7 +67,8 @@ product policy are in [`docs/MATCHMAKING.md`](docs/MATCHMAKING.md). publication now requires an allocated endpoint and verifies canonical join-authorisation signatures before exposing player rows; `cmd/allocator` now polls these claims and drives provider allocation/reconciliation/binding; - interrupted-provider reconciliation and live Agones integration remain. + durable post-provider recovery avoids a second allocation after a bind crash; + unknown provider-outcome reconciliation and live Agones integration remain. - [ ] **IN PROGRESS:** Run the Go control plane against PostgreSQL/Redis with independently runnable API, matcher, allocator and maintenance roles. The `cmd/control-plane` API role now opens PostgreSQL, applies migrations, wires @@ -79,8 +80,8 @@ product policy are in [`docs/MATCHMAKING.md`](docs/MATCHMAKING.md). allocating matches and invokes Agones through the durable allocator boundary; `cmd/maintenance` now runs bounded ranked-season rollover batches with signal-bound shutdown; - interrupted-provider reconciliation, Redis failover and live service checks - remain. + unknown provider-outcome reconciliation, Redis failover and live service + checks remain. - [ ] **IN PROGRESS:** Define assignment compatibility and opt-in `ServerConfig` flags whose defaults reproduce the community-server path. Allocation manifest validation now covers client build and future expiry; allocated servers now diff --git a/multiplayer-todo.md b/multiplayer-todo.md index 77b6cb34..ed1cb881 100644 --- a/multiplayer-todo.md +++ b/multiplayer-todo.md @@ -1213,7 +1213,7 @@ the local/CI/community transport, not a silent production fallback. | 8.27 `[D:8.26]` | **IN PROGRESS.** Go supervisor package provides local-safe Agones REST discovery, validates assigned address/port data, injects dynamic `SDR_LISTEN_PORT`/`SDR_IP`, performs explicit process-ready probing and Ready transition; direct mode bypasses Agones | `server/supervisor/` covers allocated/direct startup, invalid endpoint rejection, dynamic endpoint/Ready ordering and authenticated drain; allocated Godot now supplies a loopback readiness/drain control surface and `agones_sdk.gd` supplies sidecar Health/Ready/Shutdown/annotation REST operations; metadata watch, real Agones annotation/shutdown confirmation and emulator integration remain | | 8.28 `[D:8.6,8.27]` | **IN PROGRESS.** Supervisor separates explicit process-ready from Agones Ready and never scrapes stdout; allocated mode refuses to mark Ready without a configured readiness probe | `server/supervisor/` tests prove Ready follows the probe and direct mode remains functional; `server_control.gd`, `agones_sdk.gd` and process-level smokes prove loopback `/ready`, `/health`, bearer-protected `/drain`, sidecar-shaped Health/Ready calls and drain admission fencing; detached-container and Health-reclaim integration remain | | 8.29 `[D:8.26,8.27]` | **IN PROGRESS.** Supervisor discovers and validates the Agones endpoint, propagates the actual dynamic `--port`, and exports `SDR_LISTEN_PORT`/`SDR_IP` only for Hosted-SDR while preserving an isolated ENet path | `server/supervisor/` tests cover invalid address/port rejection, dynamic port argument/env propagation and SDR-vs-ENet separation; real Agones dynamic/passthrough mapping, POP/cert/firewall/NAT and multi-match fixture remain | -| 8.30 `[D:8.18,8.26,8.28,8.29]` | **IN PROGRESS.** Pure Go allocator filters Ready servers by region/build/protocol/transport, atomically claims one with idempotent allocation replay, verifies unanimous accepted-proposal/playlist/participant invariants before provider invocation, and now owns the assignment-publication boundary; PostgreSQL adds durable GameServer registration and compatible `SKIP LOCKED` claims with request-digest fencing, plus a leased `ALLOCATING`-match claim that derives the immutable compatibility tuple and fences bind/release by deterministic allocation ID; `server/agones` submits and validates namespaced `GameServerAllocation` responses and dynamic endpoints; `server/allocator` reconciles provider success into durable state before exposing the endpoint; `cmd/allocator` is a signal-bound polling role that drives the lease → provider → durable-record → match-bind sequence | `server/domain/allocator.go`, `server/store/allocator_sql.go`, `server/store/allocation_match_sql.go`, `server/store/allocation_match_adapter.go`, `server/agones/allocation.go`, `server/allocator/service.go`, `server/allocator/worker.go`, `server/cmd/allocator`, `server/migrations/0004_allocator_registry.sql`, `0006_match_allocation_claims.sql` and tests cover deterministic compatible selection, exhaustion, conflicting/identical allocation replay, allocation-match lease recovery/bind fencing, immutable Fleet selector labels, provider ambiguity lease retention, unknown allocations, SQL claim ordering, invalid input, provider error/malformed response/IPv6 endpoint handling, accepted-proposal gating, durable-reconciliation failure isolation and assignment replay/conflict; `TestPostgreSQLAllocatorClaimReplayAndCapacityFence` and `TestPostgreSQLAllocationMatchClaimLeaseAndBindFence` cover the live database paths when the disposable database gate is run; interrupted-provider reconciliation, signed roster metadata, bounded cross-replica retry and live Agones integration remain | +| 8.30 `[D:8.18,8.26,8.28,8.29]` | **IN PROGRESS.** Pure Go allocator filters Ready servers by region/build/protocol/transport, atomically claims one with idempotent allocation replay, verifies unanimous accepted-proposal/playlist/participant invariants before provider invocation, and now owns the assignment-publication boundary; PostgreSQL adds durable GameServer registration and compatible `SKIP LOCKED` claims with request-digest fencing, plus a leased `ALLOCATING`-match claim that derives the immutable compatibility tuple and fences bind/release by deterministic allocation ID; `server/agones` submits and validates namespaced `GameServerAllocation` responses and dynamic endpoints; `server/allocator` reconciles provider success into durable state before exposing the endpoint; `cmd/allocator` is a signal-bound polling role that drives the lease → provider → durable-record → match-bind sequence and rebinds a recovered durable provider allocation without a second provider call | `server/domain/allocator.go`, `server/store/allocator_sql.go`, `server/store/allocation_match_sql.go`, `server/store/allocation_match_adapter.go`, `server/agones/allocation.go`, `server/allocator/service.go`, `server/allocator/worker.go`, `server/cmd/allocator`, `server/migrations/0004_allocator_registry.sql`, `0006_match_allocation_claims.sql` and tests cover deterministic compatible selection, exhaustion, conflicting/identical allocation replay, allocation-match lease recovery/bind fencing, recovered-allocation binding without provider recall, immutable Fleet selector labels, provider ambiguity lease retention, unknown allocations, SQL claim ordering, invalid input, provider error/malformed response/IPv6 endpoint handling, accepted-proposal gating, durable-reconciliation failure isolation and assignment replay/conflict; `TestPostgreSQLAllocatorClaimReplayAndCapacityFence` and `TestPostgreSQLAllocationMatchClaimLeaseAndBindFence` cover the live database paths when the disposable database gate is run; unknown provider-outcome reconciliation, signed roster metadata, bounded cross-replica retry and live Agones integration remain | | 8.31 `[D:8.9,8.30]` | **IN PROGRESS.** Pure Go assignment gate requires Allocated state, exact allocation ID/match/server/region/build/protocol/transport compatibility, non-empty hosted endpoint and verified manifest signature before exposure; allocator publication cannot expose Ready state; durable roster persistence now verifies each canonical join-authorisation signature before publishing player rows; allocator service gates roster publication on allocated state and endpoint presence | `server/domain/assignment.go`, `allocator.go`, `store/assignment_sql.go` plus adversarial fixtures cover early-connect, tampered signature/manifest, wrong compatibility, empty endpoint, unknown allocation, forged roster signature, valid signature, premature publication and post-publication mutation rejection; Agones metadata watch, hosted-address registration, production signer and client-ticket publication remain | | 8.32 `[D:8.2,8.26,8.30]` | **IN PROGRESS.** Provider-neutral FleetAutoscaler baseline preserves a two-process Ready buffer, caps warm capacity, and leaves Allocated scale-down independent of the Ready floor; Fleet image references remain digest-pinned for current/rollback pre-pull | `deploy/k8s/base/fleet-autoscaler.yaml` and manifest tests cover Fleet ownership, Buffer policy and floor/cap invariants; regional on-demand node pools/failure domains, pre-pull rollout, warm-allocation p95/p99 and N+1 certification remain | | 8.33 `[D:8.26,8.32]` | **IN PROGRESS.** Fleet scheduling now requires on-demand capacity and spreads Ready processes across zones with skew 1; the autoscaler preserves the two-process Ready floor | `deploy/k8s/base/fleet.yaml` and manifest tests reject interruptible placement and single-zone concentration structurally; regional node pools, forced node-loss testing and measured N+1 headroom remain | diff --git a/server/allocator/worker.go b/server/allocator/worker.go index 9c8e858e..bf86de21 100644 --- a/server/allocator/worker.go +++ b/server/allocator/worker.go @@ -13,6 +13,7 @@ import ( // lease a match before returning it and fence binding by allocation ID. type MatchClaimSource interface { ClaimAllocatingMatch(context.Context, time.Time) (domain.AllocationRequest, bool, error) + FindProviderAllocation(context.Context, domain.AllocationRequest) (domain.Allocation, bool, error) BindAllocatedMatch(context.Context, domain.Allocation) error } @@ -37,11 +38,18 @@ func (w Worker) RunOnce(ctx context.Context) (bool, error) { if err != nil || !found { return found, err } - result, err := w.Service.Allocate(ctx, request, AllocationLabels(request)) + allocation, recorded, err := w.Claims.FindProviderAllocation(ctx, request) if err != nil { - return true, fmt.Errorf("allocate claimed match %s: %w", request.MatchID, err) + return true, fmt.Errorf("recover allocation for match %s: %w", request.MatchID, err) } - if err := w.Claims.BindAllocatedMatch(ctx, result.Allocation); err != nil { + if !recorded { + result, err := w.Service.Allocate(ctx, request, AllocationLabels(request)) + if err != nil { + return true, fmt.Errorf("allocate claimed match %s: %w", request.MatchID, err) + } + allocation = result.Allocation + } + if err := w.Claims.BindAllocatedMatch(ctx, allocation); err != nil { return true, fmt.Errorf("bind allocated match %s: %w", request.MatchID, err) } return true, nil diff --git a/server/allocator/worker_test.go b/server/allocator/worker_test.go index 739e12d1..1c35251a 100644 --- a/server/allocator/worker_test.go +++ b/server/allocator/worker_test.go @@ -12,11 +12,17 @@ import ( ) type matchClaimSpy struct { - request domain.AllocationRequest - found bool - err error - bound domain.Allocation - bindErr error + request domain.AllocationRequest + found bool + err error + recorded domain.Allocation + recordErr error + bound domain.Allocation + bindErr error +} + +func (s *matchClaimSpy) FindProviderAllocation(_ context.Context, _ domain.AllocationRequest) (domain.Allocation, bool, error) { + return s.recorded, s.recorded.AllocationID != "", s.recordErr } func (s *matchClaimSpy) ClaimAllocatingMatch(_ context.Context, _ time.Time) (domain.AllocationRequest, bool, error) { @@ -51,6 +57,18 @@ func TestWorkerRetainsClaimWhenProviderOutcomeIsAmbiguous(t *testing.T) { } } +func TestWorkerRecoversDurableProviderAllocationWithoutCallingProvider(t *testing.T) { + request := domain.AllocationRequest{AllocationID: "allocation-1", MatchID: "match-1", Region: "EU", Build: "build-1", Protocol: 1, Transport: "enet"} + recorded := domain.Allocation{AllocationID: request.AllocationID, MatchID: request.MatchID, ServerID: "server-1", Region: request.Region, Build: request.Build, Protocol: request.Protocol, Transport: request.Transport, State: domain.ServerAllocated} + claims := &matchClaimSpy{request: request, found: true, recorded: recorded} + provider := &providerSpy{} + worker := Worker{Claims: claims, Service: Service{Provider: provider, Durable: &durableSpy{}, Now: func() time.Time { return time.Unix(1_000, 0) }}, Now: func() time.Time { return time.Unix(1_000, 0) }} + processed, err := worker.RunOnce(context.Background()) + if err != nil || !processed || provider.calls != 0 || claims.bound != recorded { + t.Fatalf("processed=%t err=%v provider=%d bound=%+v", processed, err, provider.calls, claims.bound) + } +} + func TestWorkerDoesNothingWhenNoDurableMatchIsAvailable(t *testing.T) { claims := &matchClaimSpy{} worker := Worker{Claims: claims, Now: func() time.Time { return time.Unix(1_000, 0) }} diff --git a/server/store/allocation_match_adapter.go b/server/store/allocation_match_adapter.go index bc0071fe..c8385250 100644 --- a/server/store/allocation_match_adapter.go +++ b/server/store/allocation_match_adapter.go @@ -20,6 +20,10 @@ func (s AllocatingMatchClaims) ClaimAllocatingMatch(ctx context.Context, now tim return item.Request, found, err } +func (s AllocatingMatchClaims) FindProviderAllocation(ctx context.Context, request domain.AllocationRequest) (domain.Allocation, bool, error) { + return FindProviderAllocation(ctx, s.DB, request) +} + func (s AllocatingMatchClaims) BindAllocatedMatch(ctx context.Context, allocation domain.Allocation) error { return BindAllocatedMatch(ctx, s.DB, allocation) } diff --git a/server/store/allocation_match_sql.go b/server/store/allocation_match_sql.go index 6a844fd0..fd4054ca 100644 --- a/server/store/allocation_match_sql.go +++ b/server/store/allocation_match_sql.go @@ -1,6 +1,7 @@ package store import ( + "bytes" "context" "database/sql" "fmt" @@ -47,6 +48,30 @@ const ReleaseAllocatedMatchClaimSQL = `UPDATE matches SET allocation_id = NULL, allocation_claimed_at = NULL WHERE match_id = $1 AND state = 'ALLOCATING' AND allocation_id = $2 AND server_id IS NULL` +// FindProviderAllocation verifies whether a recovered lease has already +// crossed the durable provider boundary. A worker can then bind it without +// issuing a second external allocation request after a crash. +func FindProviderAllocation(ctx context.Context, db *sql.DB, request domain.AllocationRequest) (domain.Allocation, bool, error) { + if db == nil || request.AllocationID == "" || request.MatchID == "" || request.Region == "" || request.Build == "" || request.Protocol <= 0 || (request.Transport != "enet" && request.Transport != "steam_sdr") { + return domain.Allocation{}, false, fmt.Errorf("invalid provider allocation lookup") + } + var allocation domain.Allocation + var digest []byte + err := db.QueryRowContext(ctx, SelectAllocationSQL, request.AllocationID).Scan(&allocation.AllocationID, &allocation.MatchID, &allocation.ServerID, &allocation.Region, &allocation.Build, &allocation.Protocol, &allocation.Transport, &allocation.AllocatedAt, &digest) + if err == sql.ErrNoRows { + return domain.Allocation{}, false, nil + } + if err != nil { + return domain.Allocation{}, false, err + } + want := allocationRequestDigest(request) + if !bytes.Equal(digest, want[:]) || allocation.MatchID != request.MatchID || allocation.Region != request.Region || allocation.Build != request.Build || allocation.Protocol != request.Protocol || allocation.Transport != request.Transport { + return domain.Allocation{}, false, domain.ErrConflict + } + allocation.State = domain.ServerAllocated + return allocation, true, nil +} + // ClaimAllocatingMatch returns one durable provider work item. The fixed // allocation ID is retained across a lease recovery, allowing every later // reconciliation step to reject a different server for the same match.