bbangert / beaker

WSGI middleware for sessions and caching
https://beaker.readthedocs.org/
Other
517 stars 146 forks source link

ext.Database doesn't close database connections on load #146

Closed chrismcmacken closed 6 years ago

chrismcmacken commented 6 years ago

I'm using pyramid and pyramid_beaker with python 3.6 and the below configuration.

session.type = ext:database
session.auto = True
session.key = fake_local
session.data_dir = %(here)s/data/sessions/data
session.secret = some_secret
session.cookie_expires=true
session.url=mysql+pymysql://user:pass@127.0.0.1:3406/db
session.timeout=86400
#session.lock_dir=/tmp/cache/lock
session.httponly = True
session.secure = True
session.domain = 'example.com'
session.path = '/'
session.sa.pool_size = 10
session.sa.max_overflow=10

I initialize the session factory via the recommended config.include('pyramid_beaker'). After loading the page a few times SQLAlchemy is no longer able to connect to the database and throws the below exception because connections are not being closed.

sqlalchemy.exc.TimeoutError: QueuePool limit of size 10 overflow 10 reached, connection timed out, timeout 30 (Background on this error at: http://sqlalche.me/e/3o7r)

If I switch to using session.type = file the problem goes away. After some debugging I tracked it to the do_open() method in beaker.ext.DatabaseNamespaceManager. It appears that sqlalchemy.select().execute().fetchone() doesn't close the database connection in all cases.

I have submitted PR #147 with a patch for the issue.

chrismcmacken commented 6 years ago

Do you need anything else from me to consider this PR / Issue for review? I'm happy to help with testing and resolution if needed.

I would like to use the session library and I would rather not have to import my own fork of both libraries.

amol- commented 6 years ago

Sorry for the delay but I wanted to check that calling close would properly handle the connection pool and not just throw away the connection with the chance of opening/closing continously connections.

http://docs.sqlalchemy.org/en/latest/core/connections.html#basic-usage seems to confirm that closing in the context of connectionless execution does return the connection to the pool instead of closing it:

When the close() method is called, the ... connection is released to the connection pool. From the perspective of the database itself, nothing is actually “closed”, assuming pooling is in use.
chrismcmacken commented 6 years ago

@amol- No problem at all, thank you for accepting the change!