Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: ensure the transaction is legit durling the resharding #319

Merged
merged 4 commits into from
Feb 17, 2024

Conversation

supercaracal
Copy link
Member

@supercaracal supercaracal commented Feb 17, 2024

The test case is not enough and I found that the implementation of the handling replies for the transaction is corrupted.

@supercaracal
Copy link
Member Author

supercaracal commented Feb 17, 2024

https://redis.io/docs/interact/transactions/#errors-inside-a-transaction

Errors happening after EXEC instead are not handled in a special way: all the other commands will be executed even if some command fails during the transaction. It's important to note that even when a command fails, all the other commands in the queue are processed – Redis will not stop the processing of commands.

$ telnet 127.0.0.1 6379
set key3 a
+OK
multi
+OK
set key3 b
+QUEUED
incr key3
+QUEUED
exec
*2
+OK
-ERR value is not an integer or out of range
get key3
$1
b

The SET command was processed because the INCR command was queued.

multi
+OK
set key3 c
+QUEUED
mybad key3 d
-ERR unknown command 'mybad', with args beginning with: 'key3' 'd'
exec
-EXECABORT Transaction discarded because of previous errors.
get key3
$1
b

The SET command wasn't processed because of the error during the queueing.

https://redis.io/docs/interact/transactions/#what-about-rollbacks

Redis does not support rollbacks of transactions since supporting rollbacks would have a significant impact on the simplicity and performance of Redis.

It's hard to validate them perfectly in advance on client side. It seems that Redis aims to prior simplicity and performance efficiency. So I think it's wrong to use the transaction feature by complex ways. To say nothing of the cluster mode because of the CAP theorem. Redis is just a key-value store.

@supercaracal supercaracal marked this pull request as ready for review February 17, 2024 08:37
@supercaracal supercaracal merged commit c05234a into redis-rb:master Feb 17, 2024
25 checks passed
@supercaracal supercaracal deleted the fix-a-test branch February 17, 2024 08:37
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.

1 participant