Merge pull request #1255 from supercaracal/fix-cluster-tx

Fix the watch command bugs for the cluster client

Author: byroot

cluster/lib/redis/cluster.rb

@@ -96,6 +96,10 @@ def cluster(subcommand, *args)
       send_command([:cluster, subcommand] + args, &block)
     end
 
+    def watch(*keys, &block)
+      synchronize { |c| c.call_v([:watch] + keys, &block) }
+    end
+
     private
 
     def initialize_client(options)

cluster/redis-clustering.gemspec

@@ -47,5 +47,5 @@ Gem::Specification.new do |s|
   s.required_ruby_version = '>= 2.7.0'
 
   s.add_runtime_dependency('redis', s.version)
-  s.add_runtime_dependency('redis-cluster-client', '>= 0.7.0')
+  s.add_runtime_dependency('redis-cluster-client', '>= 0.7.11')
 end

cluster/test/client_transactions_test.rb

@@ -48,4 +48,25 @@ def test_cluster_client_does_not_support_transaction_by_multiple_keys
       assert_nil(redis.get("key#{i}"))
     end
   end
+
+  def test_cluster_client_does_support_transaction_with_optimistic_locking
+    redis.mset('{key}1', '1', '{key}2', '2')
+
+    another = Fiber.new do
+      cli = build_another_client
+      cli.mset('{key}1', '3', '{key}2', '4')
+      cli.close
+      Fiber.yield
+    end
+
+    redis.watch('{key}1', '{key}2') do |tx|
+      another.resume
+      v1 = redis.get('{key}1')
+      v2 = redis.get('{key}2')
+      tx.call('SET', '{key}1', v2)
+      tx.call('SET', '{key}2', v1)
+    end
+
+    assert_equal %w[3 4], redis.mget('{key}1', '{key}2')
+  end
 end

cluster/test/commands_on_transactions_test.rb

@@ -38,10 +38,29 @@ def test_unwatch
   end
 
   def test_watch
-    assert_raises(Redis::CommandError, "CROSSSLOT Keys in request don't hash to the same slot") do
-      redis.watch('key1', 'key2')
+    assert_raises(Redis::Cluster::TransactionConsistencyError) do
+      redis.watch('{key}1', '{key}2')
     end
 
-    assert_equal 'OK', redis.watch('{key}1', '{key}2')
+    assert_raises(Redis::Cluster::TransactionConsistencyError) do
+      redis.watch('key1', 'key2') do |tx|
+        tx.call('SET', 'key1', '1')
+        tx.call('SET', 'key2', '2')
+      end
+    end
+
+    assert_raises(Redis::Cluster::TransactionConsistencyError) do
+      redis.watch('{hey}1', '{hey}2') do |tx|
+        tx.call('SET', '{key}1', '1')
+        tx.call('SET', '{key}2', '2')
+      end
+    end
+
+    redis.watch('{key}1', '{key}2') do |tx|
+      tx.call('SET', '{key}1', '1')
+      tx.call('SET', '{key}2', '2')
+    end
+
+    assert_equal %w[1 2], redis.mget('{key}1', '{key}2')
   end
 end

test/helper.rb

@@ -174,7 +174,7 @@ def version
     def with_acl
       admin = _new_client
       admin.acl('SETUSER', 'johndoe', 'on',
-                '+ping', '+select', '+command', '+cluster|slots', '+cluster|nodes',
+                '+ping', '+select', '+command', '+cluster|slots', '+cluster|nodes', '+readonly',
                 '>mysecret')
       yield('johndoe', 'mysecret')
     ensure