reconnect during transaction

[ikruglov]

Found a nasty issue in the library.

Imagine:
MULTI
SET key1 1
SET key2 2
EXEC

If:

1. client connects with reconnect option
2. Redis server restarts between first and second SET

then client will successfully set key2.

[ikruglov]

the easiest way to solve this would be to disable MULTI and WATCH when in reconnect mode.
Not sure how other libraries solves the issue.

[tsee]

On 05/09/2014 03:30 PM, Ivan Kruglov wrote:

the easiest way to solve this would be to disable MULTI and WATCH when
in reconnect mode.
Not sure how other libraries solves the issue.

Or actually set flags on the client if we've seen MULTI/WATCH without
EXEC or (rollback/abort/whatever-it-is-called). And then when we
reconnect on a client during a transaction, we throw an exception or do
not reconnect. IOW "reconnect mode is disabled during transactions".

This is more work to do, but I think it is a better solution than "you
shall not have transactions on this handle at all".

--Steffen

[ikruglov]

yup, i've already started implementing this logic :-)

[trinitum]

On Fri, 09 May 2014 06:54:41 -0700
Steffen Müller notifications@github.com wrote:

On 05/09/2014 03:30 PM, Ivan Kruglov wrote:

Not sure how other libraries solves the issue.

Or actually set flags on the client if we've seen MULTI/WATCH without
EXEC or (rollback/abort/whatever-it-is-called). And then when we
reconnect on a client during a transaction, we throw an exception or
do not reconnect. IOW "reconnect mode is disabled during
transactions".

This is what RedisDB does, from the moment MULTI is sent to server and
till the moment EXEC/DISCARD is sent it will not reconnect. Although I
missed WATCH...

[dams]

Thanks for looking into this. If possible I'd like to do a third dev release with a fix to this next week.

[dams]

As a side note, I'm not too much convinced about the reconnect option usefulness. It makes some assumptions about the reconnection behavior, and doesn't provide any flexibility/configurability. Sometimes I'm tempted to dump the option altogether and dies with a proper exception and document the usage of Action::Retry or similar, to implement a real reconnect strategy. Any thought on that?