redis-rb

redis-rb copied to clipboard

what?

This change allows you to call multi without a block and queue commands successfully using Redis Transactions. Now we can use exec method to execute all commands after a redis.multi -> Fix issue https://github.com/redis/redis-rb/issues/542

why?

• Use the method redis.multi without a block raise an error as you can see in this issue https://github.com/redis/redis-rb/issues/542
• It is a good idea to have multi method that supports Redis Transactions

Caveats

A complete Redis Transaction support, will involve a big refactor of redis response handler or method yielders. This fix introduce a Response Helper but we could think a way to know when multi was activated and manage responses, errors and commands discards.

The goal of this method is add support for Redis Transaction using multi method without block however, it does not pretend to deprecated redis.multi { command; command } with block. I really prefer to use block syntax in production.

This fix works better with Redis Server 2.6.5 or greater

Changes

• Introduce Redis::Connection::ResponseHelper
• add methods that validate expected responses
• add a method that validate if response is equal to QUEUED that is the response of command under Redis multi transaction.

Uhm, we already support block-less transactions:

[2] pry(main)> r.multi
=> "OK"
[3] pry(main)> r.set "foo", "bar"
=> "QUEUED"
[4] pry(main)> r.get "foo"
=> "QUEUED"
[5] pry(main)> r.exec
=> ["OK", "bar"]

It's just that the one command, hgetall, tries to be too smart and decodes the result immediately. It should be possible to fix that command instead of building yet another indirection.

(Plus, your change looks like it will handle completely valid "QUEUED" strings the wrong way)

@badboy the issue with hgetall command is answering with the string QUEED and the Hashyfy lambdas as FLoatify one aré expecting an array or object that response to each_slice method or match string.

My idea was to handle the answer in the hgetall method however as you know the client.call send the answer back to the passed block and many methods passed through it. Which means that if block does not know how to handle the QUEUED string return by the Redis transaction many methods are going to fail after call multi method.

However I really believe that if we want to keep the support for multi without block we should implement a response handler or deprecated this syntax in favor of block sintax

@badboy the change affects only to floatify and hashify lambda handlers of client.call response. Which means that fix hgetall method a many more that use these handlers. I considered the change is unobtrusive with the existing code and fix those unexpected response.

require 'redis'
=> true
irb(main):002:0> redis = Redis.new
=> #<Redis client v3.3.0 for redis://127.0.0.1:6379/0>
irb(main):003:0> redis.multi
=> "OK"
irb(main):004:0> redis.set 'a', 1
=> "QUEUED"
irb(main):005:0> redis.hgetall 'a'
=> "QUEUED"
irb(main):006:0> redis.exec
=> ["OK", #<Redis::CommandError: WRONGTYPE Operation against a key holding the wrong kind of value>]
irb(main):007:0> redis.multi
=> "OK"
irb(main):008:0> redis.hset 'ha', 'k', 'value'
=> false
irb(main):009:0> redis.exec
=> [1]
irb(main):010:0> redis.multi
=> "OK"
irb(main):011:0> redis.hgetall 'ha'
=> "QUEUED"
irb(main):012:0> redis.exec
=> [["k", "value"]]
irb(main):013:0> 

I can't see the travis details @badboy but locally bundle exec rake run the tests passed

Tests are failing because you're using return in the lambda, which at least on <2.3 seems to be not allowed.

Apart from that, I'm still not sure this is the best solution right now.

@badboy I replaced the return statement from the lambdas in favor of a readable and supported code version for previous ruby versions of ruby 2 that prevents the LocalJumpError and allows the user understand that lambdas are clearly validating the expected response.

Floatify and Hashify handlers are now asking what they need in early stages. Now they ask to the received object, are you a:

• command_queued_response?
• string/array_expected_response?

These validations implement a port security checkpoints that increase the reliability of these methods because they clear communicating to the given object:

* The message they are expecting
* The protocols the object should know to pass trough.

I ran some test over several ruby versions:

ruby 1.8.7, ruby 1.9.3-rc1, 2.0.0-p247, 2.1.0-preview1

The hgetall works and behaves as the set, get commands; returning 'QUEUED' response when multi is activated.

irb(main):001:0> require 'redis'
=> true
irb(main):002:0> redis = Redis.new
=> #<Redis client v3.3.1 for redis://127.0.0.1:6379/0>
irb(main):003:0> redis.multi
=> "OK"
irb(main):004:0> redis.hgetall 'myhash'
=> "QUEUED"
irb(main):005:0> redis.exec
=> [[]]
irb(main):006:0> exit

I ran the test over the same ruby versions I mentioned above and the tests passed with errors as: Error running mock server: Connection reset by peer also I checked the travis builds and found the errors or exceptions as: no such file to load -- fiber (LoadError), NameError: undefined method '__current__' for class '#<Class:0x3f4ff65b>' Do I need to build anything else?

The remaining failures are unrelated (at least the ones I checked). I will think about this PR.

Please, do not change the library version number. We will do this on release.

The blockless transaction support is removed in 5.0, so closing this as no longer relevant.