fix(api): prune stale team index entries when ZSET orphans are swept - #3567
fix(api): prune stale team index entries when ZSET orphans are swept#3567AdaAibaby wants to merge 2 commits into
Conversation
When ExpiredItems found an orphaned ZSET member (sandbox key already gone), it removed the ZSET member but left the sandboxID sitting in the per-team SET index. These stale entries accumulate indefinitely: TeamItems silently skips nil MGET results, the healer only fills missing ZSET members, and Remove only cleans the team index on the normal removal path. Fix: when an orphaned ZSET member is confirmed absent (nil MGET result), SREM the sandboxID from the team index in the same sweep. The MGET has already confirmed the key is gone, so we cannot accidentally unindex a live sandbox. A concurrent Add that writes the key after our MGET will SADD the sandboxID back, so the worst outcome is a brief gap in TeamItems results for one eviction cycle. The memberRef struct gains a teamID field so the orphan handler has the context it needs without restructuring the surrounding loop. Also replace the addSandboxScript and removeSandboxScript Lua scripts with TxPipelined: they contained no conditional logic that required Lua atomicity, and MULTI/EXEC is sufficient for these cluster-safe key pairs.
70a3a35 to
705fba5
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70a3a3552c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
leonmeijer
left a comment
leonmeijer
left a comment
There was a problem hiding this comment.
ExpiredItems performs the missing-key MGET and the team-index SREM as separate operations. A resume can Add a new execution under the same sandbox ID between them, after which this cleanup removes the live sandbox from its team index permanently. Recheck key absence and remove the index member atomically in one same-slot transaction or script.
3 participants
#3566
When ExpiredItems found an orphaned ZSET member (sandbox key already gone), it removed the ZSET member but left the sandboxID sitting in the per-team SET index. These stale entries accumulate indefinitely: TeamItems silently skips nil MGET results, the healer only fills missing ZSET members, and Remove only cleans the team index on the normal removal path.
Fix: when an orphaned ZSET member is confirmed absent (nil MGET result), SREM the sandboxID from the team index in the same sweep. The MGET has already confirmed the key is gone, so we cannot accidentally unindex a live sandbox. A concurrent Add that writes the key after our MGET will SADD the sandboxID back, so the worst outcome is a brief gap in TeamItems results for one eviction cycle.
The memberRef struct gains a teamID field so the orphan handler has the context it needs without restructuring the surrounding loop.
Also replace the addSandboxScript and removeSandboxScript Lua scripts with TxPipelined: they contained no conditional logic that required Lua atomicity, and MULTI/EXEC is sufficient for these cluster-safe key pairs.
/cc @jakubno @dobrac @ValentaTomas @arkamar @tvi @tomassrnka Looking forward to your code review.