Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle ConnectionLost errors in txredis.client.RedisClient.connectionLost() #53

Open
isislovecruft opened this issue Jul 8, 2014 · 0 comments

Comments

@isislovecruft
Copy link

There is a problem where, after a txredis.client.RedisClient has successfully sent a QUIT command to the Redis server, the server properly tears down the connection, and Twisted properly calls txredis.client.RedisClient.connectionLost, which calls txredis.RedisClient.failRequests, the later of which propagates the twisted.internet.error.ConnectionLost to all requests in its internal queue (txredis.RedisClient._request_queue), without seemingly bothering to check if these requests have already succeeded. This causes the deferred transactions in the internal queue to assume that their overarching RedisClient has a half-terminated TCP connection, causing thousands of errbacks to get propagated for no good reason.

Technically, the ConnectionLost is not really an error, it's just a signal sent from Twisted that the other end tore down the connection. This is precisely what is supposed to happen when you send QUIT, so this is somewhat a design flaw in txredis.

For a piece of software I develop at work, I have fixed this issue by subclassing RedisClient and overriding the connectionLost method as follows:

class MyRedisClient(txredis.client.RedisClient):
    def connectionLost(self, reason):
        trapped = reason.trap(ConnectionLost)
        if trapped:
            log.msg(
                ("Trap prevented ConnectionLost from propagating to "
                 "`RedisClient.failRequests()`. All's well."))
        else:
            super(MyRedisClient, self).connectionLost(reason)

If desired, I can gladly either fork or provide git patches, if you think that this issue merits fixing.

Thanks for creating txredis!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant