Skip to content

Pool.close() does not close the pool gracefully #290

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

Closed
tomalexander opened this issue May 16, 2018 · 1 comment · Fixed by #299
Closed

Pool.close() does not close the pool gracefully #290

tomalexander opened this issue May 16, 2018 · 1 comment · Fixed by #299

Comments

@tomalexander
Copy link

  • asyncpg version: 0.15.0
  • PostgreSQL version: 9.5.10
  • Do you use a PostgreSQL SaaS? If so, which? Can you reproduce
    the issue with a local PostgreSQL install?
    : using local
  • Python version: 3.6.5
  • Platform: Arch Linux
  • Do you use pgbouncer?: No
  • Did you install asyncpg with pip?: Yes
  • If you built asyncpg locally, which version of Cython did you use?:
  • Can the issue be reproduced under both asyncio and
    uvloop?
    : Yes

The documentation for Pool.close() says that it "Gracefully close all connections in the pool." however if any connection is open then it throws an exception which seems not graceful. I've pasted a minimal example below demonstrating the issue but I believe its caused by Pool.close() setting self._closed immediately, which makes any call to Pool.release() fail because it immediately calls _check_init() which checks the value of the self._closed boolean and throws an exception.

Minimal Example

#!/usr/bin/env python3
#

import asyncio

from asyncpg import create_pool


async def get_pool():
    new_pool = await create_pool(
        'postgresql://localhost:5432/postgres',
        min_size=1,
        max_size=4,
        loop=loop,
    )
    return new_pool


async def hold_connection_open():
    async with pool.acquire() as connection:
        async with connection.transaction():
            connection_has_been_opened.set_result(True)
            asyncio.sleep(3)


async def wait_then_close():
    await connection_has_been_opened
    await pool.close()


loop = asyncio.get_event_loop()
pool = loop.run_until_complete(get_pool())
connection_has_been_opened = asyncio.Future()

asyncio.ensure_future(hold_connection_open())
loop.run_until_complete(wait_then_close())

Output from running example

$ ./bug.py
Task exception was never retrieved
future: <Task finished coro=<hold_connection_open() done, defined at ./bug.py:21> exception=InterfaceError('pool is closed',)>
Traceback (most recent call last):
  File "./bug.py", line 25, in hold_connection_open
    asyncio.sleep(3)
  File "/home/talexander/.virtualenvs/dynamic_async_postgres/lib/python3.6/site-packages/asyncpg/transaction.py", line 91, in __aexit__
    await self.__commit()
  File "/home/talexander/.virtualenvs/dynamic_async_postgres/lib/python3.6/site-packages/asyncpg/transaction.py", line 179, in __commit
    await self._connection.execute(query)
  File "/home/talexander/.virtualenvs/dynamic_async_postgres/lib/python3.6/site-packages/asyncpg/connection.py", line 238, in execute
    return await self._protocol.query(query, timeout)
  File "asyncpg/protocol/protocol.pyx", line 319, in query
  File "asyncpg/protocol/protocol.pyx", line 796, in asyncpg.protocol.protocol.BaseProtocol._dispatch_result
asyncpg.exceptions._base.InternalClientError: got result for unknown protocol state 3

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./bug.py", line 25, in hold_connection_open
    asyncio.sleep(3)
  File "/home/talexander/.virtualenvs/dynamic_async_postgres/lib/python3.6/site-packages/asyncpg/pool.py", line 583, in __aexit__
    await self.pool.release(con)
  File "/home/talexander/.virtualenvs/dynamic_async_postgres/lib/python3.6/site-packages/asyncpg/pool.py", line 497, in release
    self._check_init()
  File "/home/talexander/.virtualenvs/dynamic_async_postgres/lib/python3.6/site-packages/asyncpg/pool.py", line 544, in _check_init
    raise exceptions.InterfaceError('pool is closed')
asyncpg.exceptions._base.InterfaceError: pool is closed
@elprans
Copy link
Member

elprans commented May 17, 2018

I think close() should wait for all connections to be release()-d before shutting down the pool. Violent shutdown should be done with Pool.terminate().

elprans added a commit that referenced this issue May 30, 2018
Currently, `pool.close()`, despite the "graceful" designation, closes
all connections immediately regardless of whether they are acquired.

With this change, pool will wait for connections to actually be released
before closing.

WARNING: This is a potentially incompatible behavior change, as sloppily
written code which does not release acquired connections will now cause
`pool.close()` to hang forever.

Also, when `conn.close()` or `conn.terminate()` are called directly
on an acquired connection, the associated pool item is released
immediately.

Closes: #290
elprans added a commit that referenced this issue May 30, 2018
Currently, `pool.close()`, despite the "graceful" designation, closes
all connections immediately regardless of whether they are acquired.

With this change, pool will wait for connections to actually be released
before closing.

WARNING: This is a potentially incompatible behavior change, as sloppily
written code which does not release acquired connections will now cause
`pool.close()` to hang forever.

Also, when `conn.close()` or `conn.terminate()` are called directly
on an acquired connection, the associated pool item is released
immediately.

Closes: #290
elprans added a commit that referenced this issue May 31, 2018
Currently, `pool.close()`, despite the "graceful" designation, closes
all connections immediately regardless of whether they are acquired.

With this change, pool will wait for connections to actually be released
before closing.

WARNING: This is a potentially incompatible behavior change, as sloppily
written code which does not release acquired connections will now cause
`pool.close()` to hang forever.

Also, when `conn.close()` or `conn.terminate()` are called directly
on an acquired connection, the associated pool item is released
immediately.

Closes: #290
elprans added a commit that referenced this issue May 31, 2018
Currently, `pool.close()`, despite the "graceful" designation, closes
all connections immediately regardless of whether they are acquired.

With this change, pool will wait for connections to actually be released
before closing.

WARNING: This is a potentially incompatible behavior change, as sloppily
written code which does not release acquired connections will now cause
`pool.close()` to hang forever.

Also, when `conn.close()` or `conn.terminate()` are called directly
on an acquired connection, the associated pool item is released
immediately.

Closes: #290
elprans added a commit that referenced this issue May 31, 2018
Currently, `pool.close()`, despite the "graceful" designation, closes
all connections immediately regardless of whether they are acquired.

With this change, pool will wait for connections to actually be released
before closing.

WARNING: This is a potentially incompatible behavior change, as sloppily
written code which does not release acquired connections will now cause
`pool.close()` to hang forever.

Also, when `conn.close()` or `conn.terminate()` are called directly
on an acquired connection, the associated pool item is released
immediately.

Closes: #290
elprans added a commit that referenced this issue May 31, 2018
Currently, `pool.close()`, despite the "graceful" designation, closes
all connections immediately regardless of whether they are acquired.

With this change, pool will wait for connections to actually be released
before closing.

WARNING: This is a potentially incompatible behavior change, as sloppily
written code which does not release acquired connections will now cause
`pool.close()` to hang forever.

Also, when `conn.close()` or `conn.terminate()` are called directly
on an acquired connection, the associated pool item is released
immediately.

Closes: #290
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

Successfully merging a pull request may close this issue.

2 participants