Fix redis connection pool sharing. #22

Merged
merged 1 commit into from Nov 24, 2013

Projects

None yet

2 participants

@dairiki
dairiki commented Nov 23, 2013

This fixes things so that redis connection pools are shared between RedisManager instances.

I'm pretty sure this was the original intent of the code. AFAICT, RedisManager.open_connection gets called exactly once in the normal lifecycle of a RedisManager. This means the currently the collection_pools dict always get exactly one (newly created) pool stored in it (and thus is mostly pointless.)

When using beaker sessions, a new namespace manager is created for each request, which means that currently a new connection is made to the redis server for each request. This fixes that.

@didip
Owner
didip commented Nov 23, 2013

Is this all that needs to be changed?

Changing instance variable to class variable definitely require more changes than just this commit.

@dairiki
dairiki commented Nov 23, 2013

Yes, I think that's all that need to be changed. Changing connection_pools from an instance to a class attribute makes it so that RedisManager.open_connection will reuse an existing ConnectionPool if one exists for the same (host, port, db) triple.

I am currently using this in development. The patch reduced the number of open connections from my app (under low load) to the redis-server from somewhere in the twenties to one. So it definitely does do something.

As the code stands currently (without this patch), a new ConnectionPool, and thus a new TCP connection is created for each RedisManager. There is no connection sharing. Connection_pools seems to be pointless
as an instance attribute — I think it was meant to be a class attribute (or a module global) all along.

Changing instance variable to class variable definitely require more changes than just this commit.

I’m not sure that I understand. What did you have in mind?

@didip didip merged commit 2792fd7 into didip:master Nov 24, 2013
@didip
Owner
didip commented Nov 24, 2013

Sorry, I've been sick for a while and my mind is not in the right place when I made that comment.

@dairiki
dairiki commented Nov 24, 2013

NP, we’ve all been there. Feel better soon!

@dairiki dairiki deleted the unknown repository branch Nov 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment