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

options='-c statement_timeout=5000' not honoured with SSL timeout #944

Open
stefanknechtDBE opened this issue Jul 9, 2019 · 4 comments
Open

Comments

@stefanknechtDBE
Copy link

We hung over 15 minutes, despite there being a statement timeout in effect of 5 seconds.

Connect call:
psycopg2.connect( dbname = databaseName, user = userName, host = hostName, port = 5432, connect_timeout = 5, options='-c statement_timeout=5000')

Yet, the logs show:

2019-07-08 06:00:13,932 Connection check for xyz succeeded. Sleeping..
2019-07-08 06:00:44,327 Connection check for xyz succeeded. Sleeping..
2019-07-08 06:01:15,256 Connection check for xyz succeeded. Sleeping..
2019-07-08 06:17:17,580 Failed to execute query select current_user;: (<class 'psycopg2.DatabaseError'>, DatabaseError('SSL SYSCALL error: Connection timed out\n',), <traceback object at 0x7f95f185e748>)
2019-07-08 06:17:17,780 Connected to database postgres at xyz.x.rds.amazonaws.com after 0.0245s
2019-07-08 06:17:17,793 Connection check for xyz succeeded. Sleeping..

At 06:01 Amazon RDS initiated a failover. We failed to detect that the connection is dead for over 15 minutes, while waiting for "select current user;" to return, which was set to a 5 second statement timeout.

Should the statement timeout not override the ssl timeout?

@stefanknechtDBE
Copy link
Author

This is on Ubuntu 16.04.5 LTS 4.4.0-1072-aws #82-Ubuntu
psycopg2-binary 2.7.5
Python 3.6.6

@dvarrazzo
Copy link
Member

Psycopg just passes down the parameters to the libpq. If you find they don't make sense you should mention to a postgres mailing list such as pgsql-bugs.

@bobergj
Copy link

bobergj commented Mar 12, 2020

When a wait_callback is set (psyco_green is true) PQconnectStart and PQconnectPoll is used to setup the connection.

self->pgconn = PQconnectStart(dsn);

Now, https://www.postgresql.org/docs/current/libpq-connect.html says

The connect_timeout connection parameter is ignored when using PQconnectPoll; it is the application's responsibility to decide whether an excessive amount of time has elapsed. Otherwise, PQconnectStart followed by a PQconnectPoll loop is equivalent to PQconnectdb.

Now, it's the first time I look at this code, but my impression is that psycopg doesn't setup any connection timeout-timer in the psyco_green configuration. Maybe it should?

@dvarrazzo
Copy link
Member

dvarrazzo commented Mar 12, 2020

Thank you @bobergj, good catch.

I guess we can do something around the idea of checking explicitly if the timeout had expired at every poll. It's not a minor change, it's a sort of small feature.

It's also weird how to interface. I don't feel like adding a new connection parameter, as it would create an asymmetry between sync and async/green code. We could parse the options but it's tricky as they can also come from the PGOPTIONS end var. If PQconninfo works at half connection too that's doable anyway.

Reopening to jam around that. Ideas?

@dvarrazzo dvarrazzo reopened this Mar 12, 2020
kmichel-aiven added a commit to aiven/aiven-db-migrate that referenced this issue Apr 19, 2021
The wait_select callback previously installed to enable Ctrl+C during
long queries was breaking configurable connection timeout :
psycopg/psycopg2#944

This was replaced with a more visible async connection and a manual
call to a custom wait_select with support for timeout.

The timeout mimics default libpq behavior and reads the connect_timeout
connection parameter with a fallback on PGCONNECT_TIMEOUT environment
variable (and a default of 0: no timeout).

A secondary benefit is to allow importing PGMigrate inside another
project without PGMigrate altering the global set_wait_callback.
kmichel-aiven added a commit to aiven/aiven-db-migrate that referenced this issue Apr 20, 2021
The wait_select callback previously installed to enable Ctrl+C during
long queries was breaking configurable connection timeout :
psycopg/psycopg2#944

This is replaced with a more visible async connection and a manual
call to a custom wait_select with support for timeout.

The timeout mimics default libpq behavior and reads the connect_timeout
connection parameter with a fallback on PGCONNECT_TIMEOUT environment
variable (and a default of 0: no timeout).

A secondary benefit is to allow importing PGMigrate inside another
project without PGMigrate altering the global set_wait_callback.
kmichel-aiven added a commit to aiven/aiven-db-migrate that referenced this issue Apr 20, 2021
The wait_select callback previously installed to enable Ctrl+C during
long queries was breaking configurable connection timeout :
psycopg/psycopg2#944

This is replaced with a more visible async connection and a manual
call to a custom wait_select with support for timeout.

The timeout mimics default libpq behavior and reads the connect_timeout
connection parameter with a fallback on PGCONNECT_TIMEOUT environment
variable (and a default of 0: no timeout).

A secondary benefit is to allow importing PGMigrate inside another
project without PGMigrate altering the global set_wait_callback.
kmichel-aiven added a commit to aiven/aiven-db-migrate that referenced this issue Apr 26, 2021
The wait_select callback previously installed to enable Ctrl+C during
long queries was breaking configurable connection timeout :
psycopg/psycopg2#944

This is replaced with a more visible async connection and a manual
call to a custom wait_select with support for timeout.

The timeout mimics default libpq behavior and reads the connect_timeout
connection parameter with a fallback on PGCONNECT_TIMEOUT environment
variable (and a default of 0: no timeout).

A secondary benefit is to allow importing PGMigrate inside another
project without PGMigrate altering the global set_wait_callback.
@dvarrazzo dvarrazzo added this to the 2.9 milestone May 20, 2021
@dvarrazzo dvarrazzo removed this from the 2.9 milestone Jun 16, 2021
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

No branches or pull requests

3 participants