IlyaSkriblovsky / txredisapi

non-blocking redis client for python twisted
Apache License 2.0
235 stars 91 forks source link

Multiple invocations of multi commands are failing #135

Closed mikiboy closed 4 years ago

mikiboy commented 5 years ago

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.

IlyaSkriblovsky commented 5 years ago

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?

mikiboy commented 5 years ago

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

mikiboy commented 5 years ago

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.

IlyaSkriblovsky commented 5 years ago

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)

mikiboy commented 5 years ago

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.