Fix the watch command bugs for the cluster client

[supercaracal]

• The redis-cluster-client gem had had several bugs regarding the transaction feature. They has been resolved with the version 0.7.11.
  • https://github.com/redis-rb/redis-cluster-client/issues/299
• The above release makes a portion of our test cases failure. So this pull request fixes them.
  • In the cluster client, a block is needed for the watch command because of the guarantee for a critical section with an optimistic locking. This behavior is different from the standalone client. So the cluster client overrides the watch method.

[KJTsanaktsidis]

Sorry to be a pain here, but I really do want to ask whether this is actually the right interface @supercaracal @byroot

This PR defines an interface for watch where the client code looks like

r.watch('key1', 'key2') do |tx|
   tx.set 'key1', 'something in multi'
   tx.set 'key2', 'something else in multi'
end

but, ordinary not-clustered redis-rb watches look like

r.watch('key1', 'key2') do
    r.multi do |tx|
        tx.set 'key1', 'something in multi'
        tx.set 'key2', 'something else in multi'
    end
end

There are a few problems with what has been merged in my opinion:

• It is a different interface to not-clustered redis, so it makes it much harder for the same code to work in clustered and non-clustered redis
• there is no way in this interface to perform a WATCH, read a key, and then decide not to perform a transaction based on what was read and simply UNWATCH. *there is no way in this interface to perform a WATCH, read a key, and then decide to WATCH an additional key based on what was read

It requires a bit more support on the cluster-client side, but I believe the two PR's I mentioned above provide an alternative to this interface which resolves these problems and provides a better experience for users of both redis-rb/redis-cluster-client and also redis-cluster-client alone:

• https://github.com/redis/redis-rb/pull/1256
• https://github.com/redis-rb/redis-cluster-client/pull/340

WDYT? Could this be revisited?

[byroot]

@supercaracal is the redis cluster maintainer, I'm merely merging because I'm not admin of the repo so I can't really officialize this.

I have 0 interest in Redis cluster and I trust @supercaracal to maintain it.

[supercaracal]

The redis gem of version 4.x or ealier didn't support the transaction feature in the cluster mode. ALso, The redis-clustering gem of version 5.1.0 or ealier didn't work correctly for the transaction feature and the watch command. So, I thought it should be fixed first.

As you said, it broke compatibility between standalone and cluster mode, but there is a trade-off for the simplicity. I said repeatedly, the redis-cluster-client gem shouldn't be affected excessively by the redis-clustering gem. Also, I think it shouldn't be messed up with its code and design.

If you desire the same interface between standalone and cluster in the transaction feature, I think it should be implemented by the redis-clustering gem or your own adapter. At least, I don't have a plan to change the interface of the redis-cluster-client gem related to the transaction feature except any bugs.

• https://github.com/redis-rb/redis-cluster-client/issues/299

[KJTsanaktsidis]

I said repeatedly, the redis-cluster-client gem shouldn't be affected excessively by the redis-clustering gem. Also, I think it shouldn't be messed up with its code and design.

But it's not just about not being the same as redis-clustering. The redis-clustering design is the way it is because that's the shape it has to be in in order to support workflows like WATCH-then-UNWATCH or WATCH-then-WATCH-again like I said here:

• there is no way in this interface to perform a WATCH, read a key, and then decide not to perform a transaction based on what was read and simply UNWATCH.
  • there is no way in this interface to perform a WATCH, read a key, and then decide to WATCH an additional key based on what was read

These are not esoteric use-cases, a primary purpose of Redis's WATCH API is to enable mixing application logic with Redis reads and then conditionally perform an atomic sequence of writes after that.

I think it should be implemented by the redis-clustering gem

But you closed this option off by designing #watch to work in redis-clustering this way rather than the way in https://github.com/redis/redis-rb/pull/1256

or your own adapter

This isn't possible (without gratuitious amounts of send and instance_exec). We started going down this path by implementing #with, but then you started adding watch support into redis-cluster-client in a different way (with https://github.com/redis-rb/redis-cluster-client/pull/333).

At least, I don't have a plan to change the interface of the redis-cluster-client gem related to the transaction feature except any bugs.

In my humble opinion... the client library of a popular open-source database should support the full range of operations that the underlying database supports. That's worth a small amount of internal complexity in the gem.

[supercaracal]

In the redis-cluster-client side, it's already the same interface between the redis-client gem and the redis-cluster-client gem. So I don't think any additional implementation is needed.

https://github.com/redis-rb/redis-client/blob/e79e9e52c247b403eb7bdff8ff87fe006477ac7a/lib/redis_client.rb#L442-L484