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

feat: Handle database disconnects in SQL taps and targets #2257

Closed
edgarrmondragon opened this issue Feb 20, 2024 · 0 comments · Fixed by #2258
Closed

feat: Handle database disconnects in SQL taps and targets #2257

edgarrmondragon opened this issue Feb 20, 2024 · 0 comments · Fixed by #2258

Comments

@edgarrmondragon
Copy link
Collaborator

edgarrmondragon commented Feb 20, 2024

Feature scope

Taps (catalog, state, stream maps, tests, etc.)

Description

According to the SQLAlchemy docs1, we can call sa.create_engine(..., pool_pre_ping=True).

The pessimistic approach refers to emitting a test statement on the SQL connection at the start of each connection pool checkout, to test that the database connection is still viable. The implementation is dialect-specific, and makes use of either a DBAPI-specific ping method, or by using a simple SQL statement like “SELECT 1”, in order to test the connection for liveness.

The approach adds a small bit of overhead to the connection checkout process, however is otherwise the most simple and reliable approach to completely eliminating database errors due to stale pooled connections. The calling application does not need to be concerned about organizing operations to be able to recover from stale connections checked out from the pool.

Pessimistic testing of connections upon checkout is achievable by using the Pool.pre_ping argument, available from create_engine() via the create_engine.pool_pre_ping argument:

This would be need to be handled in the SQLConnector.create_engine:

return sa.create_engine(
self.sqlalchemy_url,
echo=False,
json_serializer=self.serialize_json,
json_deserializer=self.deserialize_json,
)

The docs say there's a little overhead but otherwise the change seems safe.

Related:

Footnotes

  1. https://docs.sqlalchemy.org/en/20/core/pooling.html#pool-disconnects-pessimistic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant