Skip to content

Reuse a single Redis connection in records_lib::mappack::calc_scores#129

Merged
ahmadbky merged 3 commits into
masterfrom
copilot/fix-redis-pool-starvation
May 27, 2026
Merged

Reuse a single Redis connection in records_lib::mappack::calc_scores#129
ahmadbky merged 3 commits into
masterfrom
copilot/fix-redis-pool-starvation

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 26, 2026

After the deadpool 0.12.3 upgrade reduced default pool capacity, concurrent handlers that check out Redis twice in one request could starve the pool and block unrelated Redis-backed endpoints. This PR makes pool behavior explicit and removes the deadlock-prone checkout pattern in the affected handler.

Redis pool config is now explicit and stable

Set fixed Redis pool size to 16 (instead of relying on upstream defaults).
Added a pool wait timeout of 2s for connection checkout to fail fast under saturation instead of waiting indefinitely.
Removed double-checkout pattern in player_finished

Refactored the handler to reuse the already-checked-out Redis connection for the pipeline/rank update path.
Eliminates the “hold one connection, wait for another” deadlock scenario under concurrent load.
Focused guardrail test

Added a unit test around pool config construction to assert:
max_size == 16
wait timeout == 2s

Moreover, calc_scores was checking out Redis twice in one function scope, and the second binding shadowed the first, keeping two pooled connections alive concurrently. This change keeps a single connection for the full function path.

  • Problem

    • calc_scores acquired one Redis connection for mappack membership/expiry handling, then acquired another for rank lookups in the map loop.
  • Change

    • Removed the second redis_pool.get().await? in calc_scores.
    • Reused the existing redis_conn for subsequent ranks::get_rank(...) calls.
  • Effect

    • Eliminates unnecessary concurrent pool checkout in this path.
    • Keeps connection lifetime explicit and consistent through the function.
let mut redis_conn = redis_pool.get().await?;

// ... read mappack_uids / expiry handling ...

for (i, map) in mappack.iter().enumerate() {
    // ...
    let record = RankedRecordRow {
        rank: ranks::get_rank(&mut redis_conn, map.id, record.record.time, event).await?,
        record,
    };
}

Copilot AI changed the title [WIP] Fix Redis pool starvation and deadlock issue Harden Redis pool behavior and remove per-request double checkout deadlock path May 26, 2026
Copilot AI requested a review from ahmadbky May 26, 2026 21:03
@ahmadbky ahmadbky marked this pull request as ready for review May 26, 2026 21:16
Copilot AI requested a review from ahmadbky May 26, 2026 21:19
Comment thread crates/game_api/src/http/player_finished.rs
Copilot AI changed the title Harden Redis pool behavior and remove per-request double checkout deadlock path Reuse a single Redis connection in records_lib::mappack::calc_scores May 27, 2026
@ahmadbky ahmadbky merged commit e9bf7d2 into master May 27, 2026
3 checks passed
@ahmadbky ahmadbky deleted the copilot/fix-redis-pool-starvation branch May 27, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Redis Pool Starvation and Deadlock After deadpool 0.12.3 Upgrade

2 participants