MULTI..EXEC transactions and asynchronous Call commands

[russross]

Hi,

This pull request addresses three issues:

1. When a MULTI..EXEC transaction fails because of a WATCH condition, an error is returned by ReadAll. It is difficult to test this error to see if it was due to an aborted transaction or something else. I have named the error as ErrMultiAborted, so now you can just do a check like so:

   if err == redis.ErrMultiAborted { // transaction aborted, try again

2. The Call method of an AsyncClient cannot return any errors, so I removed the err return value. It calls Write in bytes.Buffer, but the docs for that method explicitly say it will never return an err (it just has the return type to match the io.Writer interface). Removing this lets careful people know it is okay to assume the call will succeed.

3. When using WATCH with MULTI..EXEC transactions, the normal pattern is:

   WATCH foo
   GET foo
   MULTI
   do stuff using the value of foo that is now guaranteed to not change
   EXEC

AsyncClient handles the MULTI..EXEC part great, but it is a bit verbose to run the commands that happen before the MULTI is issued. This adds a SyncCall to AsyncClient that works more like Call from Client. It fails if there are already commands in the queue.

Thanks,

Russ

[simonklee]

-1 in a Multi-Bulk does not always mean that the watch condition was not met. It can also mean that the key you ask for does not exist[1].

[1] http://redis.io/topics/protocol#nil-reply

[russross]

I'm new to redis, so my apologies if I'm getting this all wrong. The documentation you referenced seems to indicate a nil reply means an empty list for a "bulk" section, not a "multi-bulk" section. In a "multi-bulk" section the nil reply means a timeout (for a blocking list action) or an aborted "multi...exec" block (my use case).

In either case, it does not invalidate the results of the other actions in the same queued sequence of instructions. Perhaps this error should be stored for the individual command ("exec" in my case) rather than returning an error for the entire sequence? I'm not sure I like that idea either, but I think the current API is lacking something.

What I need is some way to detect when a "multi...exec" block was aborted due to a "watch" condition being violated. I couldn't see any good way to detect that with the existing code. I could extract the text of the error, but that seems like a bad idea.

Any other suggestions?

[simonklee]

The documentation you referenced seems to indicate a nil reply means an empty list for a "bulk" section, not a "multi-bulk" section. In a "multi-bulk" section the nil reply means a timeout (for a blocking list action) or an aborted "multi...exec" block (my use case).

I stand corrected. I had another look. I was thinking of -1 in a mulit-bulk reply (so that would be a bulk reply). I think this would be OK, except that the error condition could happen in other circumstances as well. If we look a the example from the protocol specification;

C: BLPOP key 1
S: *-1

In this case the multi-bulik -1 is an indication of a timeout. If we change the name of the Err to something more generic, I'm OK with the the named Err.