You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "tc60045 (via GitHub)" <gi...@apache.org> on 2026/10/16 04:36:29 UTC

[I] fix(stream): potential data race between blocking XREAD/XREADGROUP and transaction [kvrocks]

tc60045 opened a new issue, #3684:
URL: https://github.com/apache/kvrocks/issues/3684

   ### Search before asking
   
   - [x] I had searched in the [issues](https://github.com/apache/kvrocks/issues) and found no similar issues.
   
   
   ### Version
   
   Linux x86_64 (Ubuntu 24.04, Proxmox LXC), RelWithDebInfo build.
   Kvrocks unstable, commit 07079ab55e.
   The same unguarded code is in the latest release, 2.17.0.
   
   ### Minimal reproduce step
   
    1. Connections A1–A4 run `XREADGROUP GROUP g c<i> COUNT 50 BLOCK 500 STREAMS s >` in a loop (blocked `XREAD ... BLOCK` readers are also exercised).
     2. Connection B loops `MULTI`, 64–200 × `ZADD`, `EXEC`.
     3. Connection C runs `XADD s * f v` every 1–20 ms, or the `XADD` is placed inside B's transaction.
   
     A gocase test with this shape, `TestPotentialDataRaceInBlockingStreamRead` (modelled on #2910's `TestPotentialDataRaceInBlockingCommand`),  fails 5 of 5 runs on unpatched `unstable`, each within 0.3–6 s. I'll include it in the PR that follows.
   
   ### What did you expect to see?
   
    Blocked stream readers are woken and served, and transactions commit normally.
   
   ### What did you see instead?
   
   Each run on unpatched `unstable` failed with a corrupted transaction write batch. The server reported:
   
     ```
     Corruption: unknown WriteBatch tag: 95
     Corruption: WriteBatch has wrong count
     Invalid argument: Merge requires ColumnFamilyOptions::merge_operator != nullptr
     ```
   
     The last error is the corrupted batch being decoded as a Merge record.
   
   ### Anything Else?
   
   **Cause.** #2910 (fixing #2900 / #2766) added `WorkConcurrencyGuard()` to `BlockingCommander::OnWrite`, so a woken blocking command can't run while an exclusive `EXEC` holds the storage in transaction mode.
   
   `CommandXRead::OnWrite` and `CommandXReadGroup::OnWrite` have their own wake-up paths and take no such guard. When an `XADD` wakes a blocked reader during another connection's `EXEC`, the reader runs its Range read on another worker against the global `txn_write_batch_`. `XREADGROUP` also writes its PEL and consumer-group update into that batch.
   
   **Possibly related:** #3338 / #3339 (`XREADGROUP BLOCK` double response under concurrent load).
   
   **Fix:** taking the guard in both `OnWrite` paths, mirroring `BlockingCommander::OnWrite`. With that change the new test passes 5 of 5, and the existing `unit/type/stream`, `unit/multi` and `unit/type/list` suites pass.
   
   ### Are you willing to submit a PR?
   
   - [x] I'm willing to submit a PR!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] fix(stream): potential data race between blocking XREAD/XREADGROUP and transaction [kvrocks]

Posted by "jihuayu (via GitHub)" <gi...@apache.org>.
jihuayu closed issue #3684: fix(stream): potential data race between blocking XREAD/XREADGROUP and transaction
URL: https://github.com/apache/kvrocks/issues/3684


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] fix(stream): potential data race between blocking XREAD/XREADGROUP and transaction [kvrocks]

Posted by "tc60045 (via GitHub)" <gi...@apache.org>.
tc60045 commented on issue #3684:
URL: https://github.com/apache/kvrocks/issues/3684#issuecomment-6208312955

   Longtime KVRocks user/evangelist storing TB of data using same at work.  
   Working on a home project to replace the data storage layer of a popular media-serving project and discovered the race condition.  I initially just added time to work past it in 2.16, but when that time started adding up to affect UX, I had to look more closely, discovering that #2900  and #2910 fixed related issues.  I figured I owed it to the community to spend some tokens patching / testing / repeat so I could give a proper patch.   I hope you won't mind the AI assist.
   
   Side note:  I don't know how you guys keep up with RocksDB's relentless pace, but you do and it is frankly pretty inspiring. Kudos and thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org