]> granicus.if.org Git - pgbouncer/commitdiff
Fix idle_transaction_timeout calculation
authorPeter Eisentraut <peter@eisentraut.org>
Mon, 24 Jun 2019 14:46:45 +0000 (16:46 +0200)
committerPeter Eisentraut <peter@eisentraut.org>
Mon, 24 Jun 2019 20:00:17 +0000 (22:00 +0200)
idle_transaction_timeout should count from the last request of the
server, because the server sent the idle information.  The code
previously used the last request of the client, which could lead to
premature timeouts.

fixes #125

src/janitor.c
test/test.sh

index 464a658993e977c1753ff917aa6e4bd8e72f2173..189de409358532d7ba037ebf1c6ea42086317539 100644 (file)
@@ -496,7 +496,7 @@ static void check_pool_size(PgPool *pool)
 static void pool_server_maint(PgPool *pool)
 {
        struct List *item, *tmp;
-       usec_t age, now = get_cached_time();
+       usec_t now = get_cached_time();
        PgSocket *server;
 
        /* find and disconnect idle servers */
@@ -514,19 +514,32 @@ static void pool_server_maint(PgPool *pool)
                }
        }
 
-       /* where query got did not get answer in query_timeout */
+       /* handle query_timeout and idle_transaction_timeout */
        if (cf_query_timeout > 0 || cf_idle_transaction_timeout > 0) {
                statlist_for_each_safe(item, &pool->active_server_list, tmp) {
+                       usec_t age_client, age_server;
+
                        server = container_of(item, PgSocket, head);
                        Assert(server->state == SV_ACTIVE);
                        if (server->ready)
                                continue;
-                       age = now - server->link->request_time;
-                       if (cf_query_timeout > 0 && age > cf_query_timeout) {
+
+                       /*
+                        * Note the different age calculations:
+                        * query_timeout counts from the last request
+                        * of the client (the client started the
+                        * query), idle_transaction_timeout counts
+                        * from the last request of the server (the
+                        * server sent the idle information).
+                        */
+                       age_client = now - server->link->request_time;
+                       age_server = now - server->request_time;
+
+                       if (cf_query_timeout > 0 && age_client > cf_query_timeout) {
                                disconnect_server(server, true, "query timeout");
                        } else if (cf_idle_transaction_timeout > 0 &&
                                   server->idle_tx &&
-                                  age > cf_idle_transaction_timeout)
+                                  age_server > cf_idle_transaction_timeout)
                        {
                                disconnect_server(server, true, "idle transaction timeout");
                        }
@@ -536,6 +549,8 @@ static void pool_server_maint(PgPool *pool)
        /* find connections that got connect, but could not log in */
        if (cf_server_connect_timeout > 0) {
                statlist_for_each_safe(item, &pool->new_server_list, tmp) {
+                       usec_t age;
+
                        server = container_of(item, PgSocket, head);
                        Assert(server->state == SV_LOGIN);
 
index 92de085fa2a2d653282319d1ec555e235f8b7825..6877d228807f69110cb55e55eb70932f929ed01b 100755 (executable)
@@ -214,6 +214,30 @@ test_query_timeout() {
        return 0
 }
 
+# idle_transaction_timeout
+test_idle_transaction_timeout() {
+       admin "set pool_mode=transaction"
+       admin "set idle_transaction_timeout=2"
+
+       psql -X --set ON_ERROR_STOP=1 p0 <<-PSQL_EOF
+       begin;
+       \! sleep 3
+       select now();
+       PSQL_EOF
+       test $? -eq 0 && return 1
+
+       # test for GH issue #125
+       psql -X --set ON_ERROR_STOP=1 p0 <<-PSQL_EOF
+       begin;
+       select pg_sleep(1);
+       \! sleep 2
+       select now();
+       PSQL_EOF
+       test $? -ne 0 && return 1
+
+       return 0
+}
+
 # client_idle_timeout
 test_client_idle_timeout() {
        admin "set client_idle_timeout=2"
@@ -553,6 +577,7 @@ test_client_idle_timeout
 test_server_lifetime
 test_server_idle_timeout
 test_query_timeout
+test_idle_transaction_timeout
 test_server_connect_timeout_establish
 test_server_connect_timeout_reject
 test_server_check_delay