Missing support for MULTI/EXEC results -- multiple errors from one command

[chloe-zen]

I'm using Redis 2.2.0rc3 with AnyEvent::Redis. From my reading of http://redis.io/topics/transactions, AE::Redis only needs one adjustment: The abillity to transmit to the client multiple error results in a single reply. Consider the below transcript:

$ telnet localhost 6379
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
SET x 1
+OK
HSET x a 1
-ERR Operation against a key holding the wrong kind of value
MULTI
+OK
HSET x a 1
+QUEUED
HSET x a 2
+QUEUED
GET x
+QUEUED
EXEC
*3
-ERR Operation against a key holding the wrong kind of value
-ERR Operation against a key holding the wrong kind of value
$1
1

Right now, when a line matching ^-ERR is found, the reader just stops, which obviously isn't OK any more. If you have an idea how you'd like to handle this, let me know. Otherwise I'll kludge something together and push it to github.

[dgl]

I'm not sure about this, I just had a go and realised it's more complex than I initially thought -- a multi/exec can contain commands in the middle that result in an error. So I think ideally it needs an API change somehow (as currently an error results in a $cv->croak).

[chloe-zen]

I agree. Perhaps ->multi needs a special case, like subscribe has, in which its parameters are a list of arrayrefs containing the subcommands to be executed. That much is straightforward enough. What to croak() with when errors occur is more complex. I suggest an object, or at least a reference, containing the array of return values (and, perforce, at least one error). How to represent the error, I'm not sure.

[chloe-zen]

I didn't mean to close this. How can it be reopened?

[dgl]

Reopened (really hate that bit of the github ui).

I'm not sure about special casing multi to take an arrayref, it seems like this might interfere with something where someone builds up a large transaction in pieces.

One thing that occurs to me is whether actually multi/exec should use the callbacks on the commands within the transaction. Currently they just see "QUEUED".

e.g. $r->multi; $r->get("foo", sub {

final return value -or- error signalled here somehow...

});
$r->exec;

The advantage I see to this is you can then wrap a $r->multi and $r->exec around some commands and it will work. Another way could be to implement it like transactions in DBIx::Class work and have multi be a txn_do a-like (but I'm not sure that would fit in totally with AnyEvent).

Again this needs API changes as currently callbacks don't see errors, the global on_error handler is the only way to tell (and this approach won't work if someone does $r->get("foo")->recv within a transaction as it won't be ready until $r->exec is called).

(I just had a look at em-redis for inspiration but it seems like it doesn't implement multi/exec -- would be interesting to see how another 'evented' Redis library implements this...)

[chloe-zen]

That's a neat idea to ensure that all the callbacks get the results from their own commands. I don't think this needs an API change per se, just a doc change -- something along the lines of "when a MULTI is active, callbacks will be deferred until EXEC time."

As for the callbacks -- I don't use the AE::Redis provided callback mechanism, it's too limited for my purposes, because I'm combining it with Tatsumaki and I need exceptions to be thrown in the context of the handlers that provoked them. Therefore I use the condvar directly, e.g. $r->get("foo")->cb(sub { eval { $_[0-]->recv }; check $@; ... }). It'd be nice if condvars were more general - croak is too specific, IMO - but one does what one can.

[chloe-zen]

I've done it :-) Check the pull req

[chloe-zen]

works for me