-
Notifications
You must be signed in to change notification settings - Fork 158
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
Pool connections recycling #373
Changes from all commits
7aae914
c622443
54eea1f
0d006db
cfc3b7e
ca36327
8f5deab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,20 +17,21 @@ | |
|
||
|
||
def create_pool(dsn=None, *, minsize=1, maxsize=10, | ||
loop=None, timeout=TIMEOUT, | ||
loop=None, timeout=TIMEOUT, pool_recycle=-1, | ||
enable_json=True, enable_hstore=True, enable_uuid=True, | ||
echo=False, on_connect=None, | ||
**kwargs): | ||
coro = _create_pool(dsn=dsn, minsize=minsize, maxsize=maxsize, loop=loop, | ||
timeout=timeout, enable_json=enable_json, | ||
enable_hstore=enable_hstore, enable_uuid=enable_uuid, | ||
echo=echo, on_connect=on_connect, **kwargs) | ||
timeout=timeout, pool_recycle=pool_recycle, | ||
enable_json=enable_json, enable_hstore=enable_hstore, | ||
enable_uuid=enable_uuid, echo=echo, | ||
on_connect=on_connect, **kwargs) | ||
return _PoolContextManager(coro) | ||
|
||
|
||
@asyncio.coroutine | ||
def _create_pool(dsn=None, *, minsize=1, maxsize=10, | ||
loop=None, timeout=TIMEOUT, | ||
loop=None, timeout=TIMEOUT, pool_recycle=-1, | ||
enable_json=True, enable_hstore=True, enable_uuid=True, | ||
echo=False, on_connect=None, | ||
**kwargs): | ||
|
@@ -40,7 +41,7 @@ def _create_pool(dsn=None, *, minsize=1, maxsize=10, | |
pool = Pool(dsn, minsize, maxsize, loop, timeout, | ||
enable_json=enable_json, enable_hstore=enable_hstore, | ||
enable_uuid=enable_uuid, echo=echo, on_connect=on_connect, | ||
**kwargs) | ||
pool_recycle=pool_recycle, **kwargs) | ||
if minsize > 0: | ||
with (yield from pool._cond): | ||
yield from pool._fill_free_pool(False) | ||
|
@@ -52,7 +53,7 @@ class Pool(asyncio.AbstractServer): | |
|
||
def __init__(self, dsn, minsize, maxsize, loop, timeout, *, | ||
enable_json, enable_hstore, enable_uuid, echo, | ||
on_connect, **kwargs): | ||
on_connect, pool_recycle, **kwargs): | ||
if minsize < 0: | ||
raise ValueError("minsize should be zero or greater") | ||
if maxsize < minsize and maxsize != 0: | ||
|
@@ -61,6 +62,7 @@ def __init__(self, dsn, minsize, maxsize, loop, timeout, *, | |
self._minsize = minsize | ||
self._loop = loop | ||
self._timeout = timeout | ||
self._recycle = pool_recycle | ||
self._enable_json = enable_json | ||
self._enable_hstore = enable_hstore | ||
self._enable_uuid = enable_uuid | ||
|
@@ -187,6 +189,10 @@ def _fill_free_pool(self, override_min): | |
conn = self._free[-1] | ||
if conn.closed: | ||
self._free.pop() | ||
elif self._recycle > -1 \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this check a bit fragile, what will happened if someone pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've used code logic from SQLAlchemy for compatibility - see: https://github.com/zzzeek/sqlalchemy/blob/master/lib/sqlalchemy/pool.py#L623 But I also think, that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make sense, lets leave |
||
and self._loop.time() - conn.last_usage > self._recycle: | ||
conn.close() | ||
self._free.pop() | ||
else: | ||
self._free.rotate() | ||
n += 1 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add check for
pool_recycle
validation, like we do forminsize
, also what do you think about making default valueNone
or large number, say 3600 seconds?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jettify
When you have database on localhost - looks like you don't need connection recycling. I wasn't able to reproduce this issue on localhost. So, large values weren't needed by default.
What about
-1
- I use this value only for compatibility with SQLAlchemy. Why did they use-1
- I don't know.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed