redis-rb / redis-cluster-client

Redis cluster-aware client for Ruby
https://rubygems.org/gems/redis-cluster-client
MIT License
21 stars 9 forks source link

Add an explicit #watch method to RedisClient::Cluster #340

Closed KJTsanaktsidis closed 7 months ago

KJTsanaktsidis commented 9 months ago

(this is a continuation of https://github.com/redis-rb/redis-cluster-client/pull/339 )


This returns a "watcher" object, which can either be used for three things:

This means that the following pattern becomes possible in redis-cluster-client:

client.watch(["{slot}k1", "{slot}k2"]) do |watcher|
  # Further reads can be performed with client directly; this is
  # perfectly safe and they will be individually redirected if required
  # as normal.
  # If a read on a slot being watched is redirected, that's also OK,
  # because it means the final EXEC will fail (since the watch got
  # modified).
  current_value = client.call('GET', '{slot}k1')
  some_other_thing = client.call('GET', '{slot}something_unwatched')

  # You can add more keys to the watch if required
  # This could raise a redireciton error, and cause the whole watch
  # block to be re-attempted
  watcher.watch('{slot}differet_key')
  different_value = client.call('GET', '{slot}different_key')

  if do_transaction?
    # Once you're ready to perform a transaction, you can use multi...
    watcher.multi do |tx|
      # tx is now a pipeliend RedisClient::Cluster::Transaction
      # instance, like normal multi
      tx.call('SET', '{slot}k1', 'new_value')
      tx.call('SET', '{slot}k2', 'new_value')
    end
    # At this point, the transaction is committed
  else
    # You can abort the transaction by calling unwatch
    # (this will also happen if an exception is thrown)
    watcher.unwatch
  end
end

This interface is what's required to make redis-clustering/redis-rb work correctly, I think.

KJTsanaktsidis commented 8 months ago

Hey @supercaracal , any chance you could have another look at this at some point? 🙏

The idea of exposing a "watcher object" is required because redis-rb needs the ability to add keys to an existing watch connection (see https://github.com/redis/redis-rb/pull/1256/files#diff-5c5f213faec136383c1e36eea915fdd563dee2dfe74ea912574e312ae097ba6bR110).

The WATCH functionality you added to the router here (https://github.com/redis-rb/redis-cluster-client/blob/23fd884823c009420066898bd7dc8370d656c707/lib/redis_client/cluster/router.rb#L323) is also not sufficient because it unconditionally attempts to commit the transaction; however, it's perfectly valid to watch some keys and then decide not to do a MULTI/EXEC at all after inspecting the contents of the keys.

Hopefully the fact that the watch methods is now private in this PR means we can merge it and experiment with it on the redis-rb side for now?

supercaracal commented 7 months ago

I don't have a plan to implement any additional interface different from redis-client. You can use the next statement for returning from your block. Other reasens are the same as my comments on your previous pull request.