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

redis connection pool size is fixed #1350

Closed
oliveagle opened this issue May 9, 2013 · 4 comments
Closed

redis connection pool size is fixed #1350

oliveagle opened this issue May 9, 2013 · 4 comments

Comments

@oliveagle
Copy link

software -> celery:3.0.19 (Chiastic Slide) kombu:2.5.10 py:2.7.3
billiard:2.7.3.28 py-amqp:N/A
platform -> system:Darwin arch:64bit imp:CPython
loader -> celery.loaders.default.Loader
settings -> transport:amqp results:disabled

we got many exceptions like this:

[2013-05-09 15:05:17,207: CRITICAL/MainProcess] Couldn't ack u'bf3950b0-5442-426a-91a5-e367a01bf13f', reason:ConnectionError('Too many connections',)
Traceback (most recent call last):
File "/usr/local/lib/python2.7/site-packages/kombu/transport/base.py", line 100, in ack_log_error
self.ack()
File "/usr/local/lib/python2.7/site-packages/kombu/transport/base.py", line 95, in ack
self.channel.basic_ack(self.delivery_tag)
File "/usr/local/lib/python2.7/site-packages/kombu/transport/virtual/init.py", line 511, in basic_ack
self.qos.ack(delivery_tag)
File "/usr/local/lib/python2.7/site-packages/kombu/transport/redis.py", line 127, in ack
self._remove_from_indices(delivery_tag).execute()
File "/usr/local/lib/python2.7/site-packages/redis/client.py", line 1802, in execute
self.shard_hint)
File "/usr/local/lib/python2.7/site-packages/redis/connection.py", line 397, in get_connection
connection = self.make_connection()
File "/usr/local/lib/python2.7/site-packages/redis/connection.py", line 404, in make_connection
raise ConnectionError("Too many connections")
ConnectionError: Too many connections

then I checkout the source code and did some simple tests:

In site-packages/redis/connection.py add 2 print line at line 404 and 405:

def make_connection(self):
    "Create a new connection"
    print "redis.connection.L404.make_connection: _cnt: %d"%(self._created_connections)
    print "redis.connection.L405.max_connections: max %d"%(self.max_connections)**
    if self._created_connections >= self.max_connections:
        raise ConnectionError("Too many connections")
    self._created_connections += 1
    return self.connection_class(**self.connection_kwargs)

and in config file add these two options:

BROKER_POOL_LIMIT = 100
CELERY_REDIS_MAX_CONNECTIONS = 20

output of L405 are always fixed.

[2013-05-09 16:19:40,441: WARNING/MainProcess] redis.connection.L404.make_connection: _cnt: 0
[2013-05-09 16:19:40,441: WARNING/MainProcess] redis.connection.L405.max_connections: 10
[2013-05-09 16:19:40,441: WARNING/MainProcess] redis.connection.L404.make_connection: _cnt: 1
[2013-05-09 16:19:40,442: WARNING/MainProcess] redis.connection.L405.max_connections: 10

then I modified site-packages/kombu/transport/redis.py Line 331.

class Channel(virtual.Channel):
    QoS = QoS
    . . .
    socket_timeout = None
    # max_connections = 10    # Line 331, this is fixed and cannot be modified  using celery configurations
   max_connections = 30

things changed afterwards.

[2013-05-09 16:53:39,499: WARNING/Beat] redis.connection.L404.make_connection: _cnt: 0
[2013-05-09 16:53:39,499: WARNING/Beat] redis.connection.L405.max_connections: 30
[2013-05-09 16:53:39,500: WARNING/Beat] redis.connection.L404.make_connection: _cnt: 1
[2013-05-09 16:53:39,500: WARNING/Beat] redis.connection.L405.max_connections: 30

finally, kombu can change max_connections when creating connections:

In [1]: from kombu import Connection
In [2]: conn = Connection("redis://")
In [3]: conn.connection
redis.connection.L404.make_connection: _cnt: 0
redis.connection.L405.max_connections: 30
redis.connection.L404.make_connection: _cnt: 1
redis.connection.L405.max_connections: 30
Out[3]: <kombu.transport.redis.Transport at 0x109b79ed0>
In [6]: conn = Connection("redis://", transport_options={"max_connections":100})
In [7]: conn.connection
redis.connection.L404.make_connection: _cnt: 0
redis.connection.L405.max_connections: 100
redis.connection.L404.make_connection: _cnt: 1
redis.connection.L405.max_connections: 100
Out[7]: <kombu.transport.redis.Transport at 0x109c56990>

is this a bug? or how I can change this in configuration file?

@ask
Copy link
Contributor

ask commented May 14, 2013

CELERY_REDIS_MAX_CONNECTIONS is a setting for the result backend, it is not used by the broker transport.

You can set that using the BROKER_TRANSPORT_OPTIONS setting:

BROKER_TRANSPORT_OPTIONS = {
    'max_connections': 20,
}

@ask ask closed this as completed May 14, 2013
@ask
Copy link
Contributor

ask commented May 14, 2013

I get that this is confusing, maybe the redis backend settings could be reorganized into a RESULT_BACKEND_OPTIONS setting.

@oliveagle
Copy link
Author

thx a lot. it works. RESULT_BACKEND_OPTIONS is a good idea.

btw, what about BROKER_POOL_LIMIT any way?

@ask
Copy link
Contributor

ask commented May 14, 2013

BROKER_POOL_LIMIT limits the number of kombu.Connection's, but the redis transport can use several actual redis connections to emulate AMQP channels. This is used e.g. to receive both task messages and broadcast commands at the same time.

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

2 participants