diff --git a/server/store/postgres_integration_test.go b/server/store/postgres_integration_test.go index 29d6ec76..291753e1 100644 --- a/server/store/postgres_integration_test.go +++ b/server/store/postgres_integration_test.go @@ -1848,3 +1848,71 @@ func TestPostgreSQLMigrationsRollBackAndReapplyCleanly(t *testing.T) { t.Fatal("reapply after rollback did not recreate the schema") } } + +// Ranked matchmaking reads Candidate.Rating for tolerance, selection scoring +// and team partitioning. The candidate projection did not join the ratings +// table and its scan never set the field, so every PostgreSQL-sourced ranked +// candidate arrived with Go's zero value and the matcher treated a 900-rated +// player as identical to a 2100-rated one. Unit tests missed this because they +// construct candidates with ratings already populated. +func TestPostgreSQLQueuedCandidatesCarryAuthoritativeRatings(t *testing.T) { + db := openIntegrationPostgres(t) + applyIntegrationMigrations(t, db) + + now := time.Now().UTC().Truncate(time.Microsecond) + ctx := context.Background() + // "rated-low" and "rated-high" are deliberately far apart; "unrated" has no + // ratings row at all and must fall back to the new-profile default. + seeded := map[string]float64{"rated-low": 900, "rated-high": 2100} + for _, playerID := range []string{"rated-low", "rated-high", "unrated"} { + if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ($1, $2)`, playerID, "steam-"+playerID); err != nil { + t.Fatal(err) + } + if rating, ok := seeded[playerID]; ok { + if _, err := db.ExecContext(ctx, `INSERT INTO ratings (player_id, rating) VALUES ($1, $2)`, playerID, rating); err != nil { + t.Fatal(err) + } + } + spec := domain.QueueSpec{Playlist: domain.Ranked, ClientBuild: "rating-build", ProtocolVersion: 1} + ticket, err := CreateQueueTicket(ctx, db, "ticket-"+playerID, playerID, "rating-create-"+playerID+"-01", spec, now) + if err != nil { + t.Fatalf("create queue ticket for %s: %v", playerID, err) + } + // The Redis projection is seeded from this candidate rather than from + // the query below, so it must carry the same rating or the two + // projections disagree about who is comparable to whom. + want := domain.GlickoInitialRating + if rating, ok := seeded[playerID]; ok { + want = rating + } + if ticket.Candidate.Rating != want { + t.Fatalf("%s enqueue candidate rating = %v, want %v", playerID, ticket.Candidate.Rating, want) + } + } + + candidates, err := ListQueuedCandidates(ctx, db, domain.Ranked, now, 100) + if err != nil { + t.Fatalf("list queued candidates: %v", err) + } + if len(candidates) != 3 { + t.Fatalf("expected 3 candidates, got %d", len(candidates)) + } + got := make(map[string]float64, len(candidates)) + for _, candidate := range candidates { + got[candidate.PlayerID] = candidate.Rating + } + for playerID, want := range map[string]float64{ + "rated-low": 900, "rated-high": 2100, "unrated": domain.GlickoInitialRating, + } { + if got[playerID] != want { + t.Fatalf("%s durable candidate rating = %v, want %v", playerID, got[playerID], want) + } + } + + // The whole point of loading the rating is that the matcher can tell these + // players apart. Assert the spread survives into team partitioning rather + // than only that the field is non-zero. + if got["rated-high"]-got["rated-low"] != 1200 { + t.Fatalf("rating spread collapsed: %v", got) + } +} diff --git a/server/store/queue_sql.go b/server/store/queue_sql.go index 4d2c2213..83d82a44 100644 --- a/server/store/queue_sql.go +++ b/server/store/queue_sql.go @@ -58,13 +58,25 @@ ORDER BY ends_at DESC LIMIT 1` ) -const QueueCandidateProjectionSQL = `SELECT ticket_id, player_id, playlist, client_build, - protocol_version, enqueued_at, predicted_rtt -FROM queue_tickets -WHERE state = 'QUEUED' AND playlist = $1 AND expires_at > $2 -ORDER BY enqueued_at, ticket_id +// QueueCandidateProjectionSQL joins the authoritative rating. Without it every +// PostgreSQL-sourced candidate carried Go's zero value, and since rating +// tolerance, selection scoring and team partitioning all read that field, +// ranked matchmaking treated every player as identically rated. A player with +// no ratings row yet is a genuinely new profile and starts at the Glicko +// initial rating, matching domain.GlickoInitialRating and the column default. +// The rating is never taken from the client. +const QueueCandidateProjectionSQL = `SELECT q.ticket_id, q.player_id, q.playlist, q.client_build, + q.protocol_version, q.enqueued_at, q.predicted_rtt, COALESCE(r.rating, $4) +FROM queue_tickets q +LEFT JOIN ratings r ON r.player_id = q.player_id +WHERE q.state = 'QUEUED' AND q.playlist = $1 AND q.expires_at > $2 +ORDER BY q.enqueued_at, q.ticket_id LIMIT $3` +// QueueTicketRatingSQL resolves a player's authoritative rating, falling back +// to the new-profile default when they have no ratings row yet. +const QueueTicketRatingSQL = `SELECT COALESCE((SELECT rating FROM ratings WHERE player_id = $1), $2)` + const RankedParticipantSQL = `SELECT player_id, steam_id FROM identities WHERE player_id = ANY($1) @@ -106,7 +118,7 @@ func ListQueuedCandidates(ctx context.Context, db *sql.DB, playlist domain.Playl if db == nil || (playlist != domain.Casual && playlist != domain.Ranked) || now.IsZero() || limit < 1 || limit > 1000 { return nil, fmt.Errorf("invalid queued candidate arguments") } - rows, err := db.QueryContext(ctx, QueueCandidateProjectionSQL, string(playlist), now, limit) + rows, err := db.QueryContext(ctx, QueueCandidateProjectionSQL, string(playlist), now, limit, domain.GlickoInitialRating) if err != nil { return nil, err } @@ -116,7 +128,7 @@ func ListQueuedCandidates(ctx context.Context, db *sql.DB, playlist domain.Playl var candidate domain.Candidate var playlist string var predictedRTT []byte - if err := rows.Scan(&candidate.TicketID, &candidate.PlayerID, &playlist, &candidate.ClientBuild, &candidate.ProtocolVersion, &candidate.EnqueuedAt, &predictedRTT); err != nil { + if err := rows.Scan(&candidate.TicketID, &candidate.PlayerID, &playlist, &candidate.ClientBuild, &candidate.ProtocolVersion, &candidate.EnqueuedAt, &predictedRTT, &candidate.Rating); err != nil { return nil, err } if err := json.Unmarshal(predictedRTT, &candidate.PredictedRTT); err != nil { @@ -138,7 +150,15 @@ func CreateQueueTicket(ctx context.Context, db *sql.DB, ticketID, playerID, idem digest := sha256.Sum256([]byte(fmt.Sprintf("%s|%s|%s|%s|%d", ticketID, playerID, spec.Playlist, spec.ClientBuild, spec.ProtocolVersion))) var ticket domain.QueueTicket err := RunSerializable(ctx, db, DefaultSerializableAttempts, func(ctx context.Context, tx *sql.Tx) error { - candidate := domain.Candidate{TicketID: ticketID, PlayerID: playerID, Playlist: spec.Playlist, ClientBuild: spec.ClientBuild, ProtocolVersion: spec.ProtocolVersion, EnqueuedAt: now} + // The Redis projection is seeded from this candidate, so it must carry + // the same authoritative rating the durable candidate query joins. + // Reading it inside the transaction keeps both projections agreeing on + // one value rather than one of them defaulting to zero. + var rating float64 + if err := tx.QueryRowContext(ctx, QueueTicketRatingSQL, playerID, domain.GlickoInitialRating).Scan(&rating); err != nil { + return err + } + candidate := domain.Candidate{TicketID: ticketID, PlayerID: playerID, Playlist: spec.Playlist, ClientBuild: spec.ClientBuild, ProtocolVersion: spec.ProtocolVersion, EnqueuedAt: now, Rating: rating} ticket = domain.QueueTicket{TicketID: ticketID, PlayerID: playerID, Candidate: candidate, Playlist: spec.Playlist, State: domain.Queued, EnqueuedAt: now, ExpiresAt: now.Add(domain.QueueExpiryWindow)} stored, err := json.Marshal(queueTicketRecordFromDomain(ticket)) if err != nil {