Skip to content

Conversation

@c-w
Copy link
Contributor

@c-w c-w commented May 31, 2018

Previously we were using optimistic disconnect handling for the database connection pool: assume that all the connections are live and recycle them before a certain timeout just in case the server kills the connections after a certain period of inactivity. This approach reduced the frequency of "MySQL went away" errors, but it turns out that it didn't eliminate them.

As such, this change switches to pessimistic disconnect handling: before using a given connection, it is first tested for liveness by SQLAlchemy. This is implemented by passing pool_pre_ping to the create_engine call (docs). Note that this option was only introduced in SQLAlchemy 1.2 (announcement) so this change also updates the SQLAlchemy dependency to the latest release.

c-w added 2 commits May 31, 2018 11:56
Previously we were using optimistic disconnect handling for the database
connection pool: assume that all the connections are live and recycle
them before a certain timeout just in case the server kills the
connections after a certain period of inactivity. This approach reduced
the frequency of "MySQL went away" errors, but it turns out that it
didn't eliminate them.

As such, this change switches to pessimistic disconnect handling: before
using a given connection, it is first tested for liveness by SQLAlchemy.
This is implemented by passing `pool_pre_ping` to the `create_engine`
call [1]. Note that this option was only introduced in SQLAlchemy 1.2
[2] so this change also updates the SQLAlchemy dependency to the latest
release.

[1] http://docs.sqlalchemy.org/en/latest/core/pooling.html#pool-disconnects-pessimistic
[2] https://twitter.com/sqlalchemy/status/884415051125452802
Copy link
Member

@colinschoen colinschoen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@colinschoen colinschoen merged commit 24bb16b into okpy:master May 31, 2018
@c-w c-w deleted the bug/c-w/pessimistic-disconnect-handling branch May 31, 2018 18:50
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 this pull request may close these issues.

3 participants