IlyaSkriblovsky/txredisapi

Multiple invocations of multi commands are failing

mikiboy opened this issue · 5 comments

If the code uses multi transactions multiple times in different functions with defer.inlinecallbacks then on high load getting errors like MULTI calls can't be nested and the value for some other commands return QUEUED or even worse of hmget commands {'Q':'U','E':'D'}.

If using a single connection this issue occurs more frequently than using connection pool but still exists.

I think this is due to the fact that in between starting and committing of transaction other redis commands gets called which resets the inMulti or inTransaction instance variables.

Seems like a serious bug. May be we are not properly blocking connection while it is in transaction mode.

Could you please try to compose some minimal example to reproduce the issue?

I have added a simple server and client applications to reproduce most of the reported errors I got.
Since these sample functions don't do much could not get the hmget error but other issues are present. Just start the server script and you will get the errors. No need to use the client script.
texredisapi_issue_17.zip

The two errors I got immediately after server start:
txredisapi.RedisError: Not in transaction
txredisapi.ResponseError: ERR MULTI calls can not be nested

One of the possible solution I can think of is to gather the multi call commands and only on the commit function execution actually transfer the commands to the redis server.
One tricky thing here would be to manage multiple invocations of the multi function itself.
If it change instance variables it's not going to work in a way I have used in the sample code.
Hope the provided samples are enough. Let me know if I can do anything else to fix this issue.

Thanks! Your example helped to identify the bug (#136).

Your code issues SMEMBERS right before MULTI and there was a bug that triggered when receiving bulk response from SMEMBERS after sending MULTI but before receiving OK reply to it.

Could you please try to reproduce the issue with the code of #136?

By the way, are you sure you really need transactions in places where you do hgetall? It looks like you use UUIDs for task IDs, so content of tasks:{task_id} is probably immutable and you probably can achieve the same result without a transaction. Instead you can save some round-trip times by pipelining requests:

deferreds = []
for task_id in task_ids:
    if task_id:
        deferreds.append(rc.hgetall(f'{REDIS_QUEUE}:tasks:{task_id}'))
tasks = yield defer.gatherResults(deferreds)

(This code send all hgetalls to server at once and then waits for all responses at once)

The code is just for an example which could make multiple multi calls before completion of another.
For my need I have updated code to not to use the MULTI commands at all.
I will have a look at the updated code and let you know.