Implement #watch and #multi specially for cluster-client#1256
Open
KJTsanaktsidis wants to merge 1 commit intoredis:masterfrom
Open
Implement #watch and #multi specially for cluster-client#1256KJTsanaktsidis wants to merge 1 commit intoredis:masterfrom
KJTsanaktsidis wants to merge 1 commit intoredis:masterfrom
Conversation
This PR makes watch & multi work more or less the same way for
clustering as they do for normal redis.
Since it's supposed to be valid to perform your multi call on the
original redis object, like this:
```
redis.watch('key') do
redis.multi do |tx|
# tx is performed on the same connection as the watch
end
end
```
we need to keeps some state in an ivar @active_watcher so we know to
call MULTI on the same actual connection as WATCH (and appropriately
fail if the keys got redirected or the node went down). This is
technically threadsafe, because the watch/multi implementation is
wrapped in the `synchronize` monitor; however, for good performance in
multithreaded environments, you will most likely want to use a
connection pool of Redis::Cluster instances.
KJTsanaktsidis
force-pushed
the
ktsanaktsidis/cluster_txn_support
branch
from
e2e2872 to
5bdb9e8
Compare
haiderRizvii
approved these changes
Apr 4, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes watch & multi work more or less the same way for clustering as they do for normal redis.
Since it's supposed to be valid to perform your multi call on the original redis object, like this:
we need to keeps some state in an ivar @active_watcher so we know to call MULTI on the same actual connection as WATCH (and appropriately fail if the keys got redirected or the node went down). This is technically threadsafe, because the watch/multi implementation is wrapped in the
synchronizemonitor; however, for good performance in multithreaded environments, you will most likely want to use a connection pool of Redis::Cluster instances.(note - this will need redis-rb/redis-cluster-client#339 to be merged in order to work. But "the tests pass on my machine, I promise" 😂 )