You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@kvrocks.apache.org by "nathanlo-hrt (via GitHub)" <gi...@apache.org> on 2025/05/23 18:06:10 UTC

[I] MULTI/EXEC commit failures improperly responded to [kvrocks]

nathanlo-hrt opened a new issue, #2992:
URL: https://github.com/apache/kvrocks/issues/2992

   ### Search before asking
   
   - [x] I had searched in the [issues](https://github.com/apache/kvrocks/issues) and found no similar issues.
   
   
   ### Version
   
   repros on most unstable trunks
   
   ### Minimal reproduce step
   
   Enable 
   ```
   rocksdb.write_options.no_slowdown no
   ```
   in the kvrocks config, and open a server. Send it lots of write load, until a simple `SET` fails with `Low priority write stall`.
   
   Then send
   ```
   MULTI
   SET a b
   EXEC
   ```
   
   ### What did you expect to see?
   
   ```
   *1
   -EXEC Low priority write stall
   ```
   
   ### What did you see instead?
   
   ```
   *1
   +OK
   -EXEC Low priority write stall
   ```
   
   
   ### Anything Else?
   
   Fundamentally, this happens because `Connection::ExecuteCommands` eagerly responds `+OK` to successful mutation commands before the WriteBatch is committed: my current workaround is to defer calling `Connection::Reply` on these replies until after the transaction is successfully committed.
   
   If the commit fails, I arbitrarily change one of the responses to an error. However, this is hacky and doesn't correctly attribute the failures to the commands that attempted a write.
   
   ### Are you willing to submit a PR?
   
   - [ ] 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] MULTI/EXEC commit failures improperly responded to [kvrocks]

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.

PragmaTwice closed issue #2992: MULTI/EXEC commit failures improperly responded to
URL: https://github.com/apache/kvrocks/issues/2992


-- 
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] MULTI/EXEC commit failures improperly responded to [kvrocks]

Posted by "git-hulk (via GitHub)" <gi...@apache.org>.

git-hulk commented on issue #2992:
URL: https://github.com/apache/kvrocks/issues/2992#issuecomment-2911273983

   > invalid RESP reply, prefixed by +N but has N + 1 elements. this is important and we should fix this - to give users valid reply in RESP (e.g. change N to N + 1).
   
   For transaction mode, it would not return the number prefix. Perhaps a possible way is to buffer the output and use the same error for each command if it fails to commit.


-- 
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] MULTI/EXEC commit failures improperly responded to [kvrocks]

Posted by "nathanlo-hrt (via GitHub)" <gi...@apache.org>.

nathanlo-hrt commented on issue #2992:
URL: https://github.com/apache/kvrocks/issues/2992#issuecomment-2913016277

   @git-hulk yes, i'm enabling the `no-slowdown` option: i'm then sending lots and lots of reads+writes, until something like `redis-cli -p <port> set key value` returns a `Low priority write stall`; then the repro should work
   
   > Perhaps a possible way is to buffer the output and use the same error for each command if it fails to commit.
   
   should we preserve the OK responses for things which don't add to the WriteBatch? that way a client can get partial results from transactions that have both `SET` and `GET`


-- 
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] MULTI/EXEC commit failures improperly responded to [kvrocks]

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.

PragmaTwice commented on issue #2992:
URL: https://github.com/apache/kvrocks/issues/2992#issuecomment-2906373127

   Good catch! I'll look into this while available.


-- 
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] MULTI/EXEC commit failures improperly responded to [kvrocks]

Posted by "git-hulk (via GitHub)" <gi...@apache.org>.

git-hulk commented on issue #2992:
URL: https://github.com/apache/kvrocks/issues/2992#issuecomment-2907669617

   @nathanlo-hrt Are you enabling the `no-slowdown` option?


-- 
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] MULTI/EXEC commit failures improperly responded to [kvrocks]

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.

PragmaTwice commented on issue #2992:
URL: https://github.com/apache/kvrocks/issues/2992#issuecomment-2908684330

   Several points:
   - invalid RESP reply, prefixed by `+N` but has `N + 1` elements. this is important and we should fix this - to give users valid reply in RESP (e.g. change N to N + 1).
   - which commands make the WriteBatch commit fail? it's quite hard to determine IMHO. So maybe we can only reply an additional error for this.


-- 
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