Use ZCOUNT for ranks computing

[ahmadbky]

Closes #106.

[Copilot]

Pull request overview

This PR refactors the rank computation system to use Redis's ZCOUNT command instead of ZRANGEBYSCORE + ZRANK, significantly simplifying the implementation by removing the complex locking mechanism and error handling infrastructure that was previously needed.

Key Changes:

• Replaced multi-step rank calculation with atomic pipeline using ZCOUNT to count records below a given time
• Removed the lock system (lock_within function) that prevented concurrent leaderboard operations
• Removed RankComputeError and all associated error handling infrastructure
• Simplified get_rank to no longer require database connection or perform leaderboard synchronization internally
• Removed update_rank function in favor of direct zadd calls

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
crates/records_lib/src/ranks.rs	Major refactor: removed locking system, simplified get_rank to use ZCOUNT in atomic pipeline, removed 200+ lines of complex synchronization code
crates/records_lib/src/error.rs	Removed RankComputeError variant from error enum
crates/records_lib/src/mappack.rs	Updated get_rank call to remove conn parameter
crates/records_lib/src/leaderboard.rs	Updated get_rank call to remove conn parameter
crates/graphql-api/src/objects/root.rs	Updated get_rank calls to remove conn parameter
crates/graphql-api/src/objects/player.rs	Updated get_rank calls to remove conn parameter
crates/graphql-api/src/objects/map.rs	Updated get_rank calls to remove conn parameter
crates/game_api/src/main.rs	Removed mask_rank_compute_error middleware
crates/game_api/src/http/player_finished.rs	Updated get_rank calls and replaced update_rank with direct zadd
crates/game_api/src/http/overview.rs	Updated get_rank call to remove conn parameter
crates/game_api/src/graphql.rs	Removed RankComputeError handling logic
crates/game_api/src/error.rs	Removed RankComputeError error code, left comment placeholder
crates/game_api/src/configure.rs	Removed mask_rank_compute_error middleware and send_compute_err_msg_detached function (~165 lines)

Comments suppressed due to low confidence (6)

crates/graphql-api/src/objects/root.rs:67

• The get_rank call here retrieves the rank without first ensuring the leaderboard is synchronized with the database. The previous implementation of get_rank would internally handle out-of-sync leaderboards, but the new implementation assumes the Redis leaderboard is already current. This can result in incorrect ranks being returned. Add a call to update_leaderboard before calling get_rank, similar to how it's done in the map queries (see map.rs lines 69 and 235).

        rank: get_rank(
            redis_conn,
            record.map_id,
            record.record_player_id,
            record.time,
            event,
        )
        .await?,

crates/graphql-api/src/objects/root.rs:306

• Each get_rank call in this loop retrieves ranks without ensuring leaderboards are synchronized. Since records can be from different maps, you'll need to either call update_leaderboard for each unique map_id before the loop, or call it inside the loop for each record's map. The previous implementation of get_rank handled synchronization internally, but the new implementation does not. This can result in incorrect ranks being returned.

        let rank = get_rank(
            redis_conn,
            record.map_id,
            record.record_player_id,
            record.time,
            event,
        )
        .await?;

crates/graphql-api/src/objects/player.rs:190

• Each get_rank call in this loop retrieves ranks without ensuring leaderboards are synchronized. Since a player's records can span multiple maps, you'll need to call update_leaderboard for each unique map_id. The previous implementation of get_rank handled synchronization internally, but the new implementation does not. This can result in incorrect ranks being returned.

        let rank = get_rank(
            redis_conn,
            record.map_id,
            record.record_player_id,
            record.time,
            event,
        )
        .await?;

crates/graphql-api/src/objects/player.rs:343

• Each get_rank call in this loop retrieves ranks without ensuring leaderboards are synchronized. Since a player's records can span multiple maps, you'll need to call update_leaderboard for each unique map_id. The previous implementation of get_rank handled synchronization internally, but the new implementation does not. This can result in incorrect ranks being returned.

        let rank = get_rank(
            redis_conn,
            record.map_id,
            record.record_player_id,
            record.time,
            event,
        )
        .await?;

crates/records_lib/src/mappack.rs:430

• Each get_rank call in this loop retrieves ranks without ensuring the leaderboard for that map is synchronized with the database. The previous implementation of get_rank would handle synchronization internally, but the new implementation does not. Consider calling update_leaderboard for each map before processing its records. This can result in incorrect ranks being returned.

                rank: get_rank(
                    redis_conn,
                    map.id,
                    record.record.record_player_id,
                    record.record.time,
                    event,
                )
                .await?,

crates/graphql-api/src/objects/root.rs:103

• Each get_rank call in this loop retrieves ranks without ensuring leaderboards are synchronized. Since records can be from different maps, you'll need to either call update_leaderboard for each unique map_id before the loop, or call it inside the loop for each record's map. The previous implementation of get_rank handled synchronization internally, but the new implementation does not. This can result in incorrect ranks being returned.

        let rank = get_rank(
            redis_conn,
            record.map_id,
            record.record_player_id,
            record.time,
            event,
        )
        .await?;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.