Skip to content

Commit

Permalink
Do not wait server_login_retry for next connect if cancellation succeeds
Browse files Browse the repository at this point in the history
If postgres restarts while there are N cancellations in the queue,
pgbouncer is currently unavailable for at least N*server_login_retry
because it uses every new connection for one queued cancellation and
then waits server_login_retry before opening a new connection because
the last_connect_failed flag is still set to 1. This can lead to
prolonged downtime.

This changes fixes the issue by introducing a last_login_failed flag.
The last_connect_failed is now reset when a cancellation succeeds, such
that launch_new_connection no longer waits if pgbouncer manages to
connect, but has queued cancellations. The last_login_failed flag has
the same semantics as the last_connect_failed flag had previously, such
that check_fast_fail still rejects connections when there are no servers
available and the last login failed.
  • Loading branch information
marcoslot authored and dimitri committed Nov 8, 2018
1 parent 7b97cb8 commit 28de1c6
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
1 change: 1 addition & 0 deletions include/bouncer.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ struct PgPool {
/* if last connect failed, there should be delay before next */
usec_t last_connect_time;
unsigned last_connect_failed:1;
unsigned last_login_failed:1;

unsigned welcome_msg_ready:1;
};
Expand Down
16 changes: 14 additions & 2 deletions src/objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -592,8 +592,8 @@ bool check_fast_fail(PgSocket *client)
int cnt;
PgPool *pool = client->pool;

/* reject if no servers and last connect failed */
if (!pool->last_connect_failed)
/* reject if no servers are available and the last login failed */
if (!pool->last_login_failed)
return true;
cnt = pool_server_count(pool) - statlist_count(&pool->new_server_list);
if (cnt)
Expand Down Expand Up @@ -760,6 +760,7 @@ bool release_server(PgSocket *server)
case SV_TESTED:
break;
case SV_LOGIN:
pool->last_login_failed = 0;
pool->last_connect_failed = 0;
break;
default:
Expand Down Expand Up @@ -832,9 +833,20 @@ void disconnect_server(PgSocket *server, bool notify, const char *reason, ...)
* except when sending cancel packet
*/
if (!server->ready)
{
pool->last_login_failed = 1;
pool->last_connect_failed = 1;
}
else
{
/*
* We did manage to connect and used the connection for query
* cancellation, so to the best of our knowledge we can connect to
* the server, reset last_connect_failed accordingly.
*/
pool->last_connect_failed = 0;
send_term = 0;
}
break;
default:
fatal("disconnect_server: bad server state (%d)", server->state);
Expand Down

0 comments on commit 28de1c6

Please sign in to comment.