Fix connection_life_time not working when serialize_accept is enabled.

If serialize_accept is enabled, pgpool child process tries to acquire
semaphore locking so that there's only one process which can issue
accept(2). Unfortunately if connection_life_time is enabled, an alarm
is set right before the semaphore locking. So when the alarm fires,
nothing happened because the process was in acquiring semaphore lock
loop by using pool_semaphore_lock().

To fix this new pool_semaphore_lock_allow_interrupt() is introduced,
which immediately returns if interrupted by a signal. The caller:
wait_for_new_connections() checks whether connection_life_time alarm
is fired. If so, call backend_timer() to close expired backend
connection cache.

Discussion: https://www.pgpool.net/pipermail/pgpool-general/2020-August/007233.html

diff --git a/src/include/pool.h b/src/include/pool.h
index 3a07a1180b11e151d74f15c371d05df06038a472..0595e5dca44fe24d6d6aa7523653b29a73d1a914 100644 (file)
--- a/src/include/pool.h
+++ b/src/include/pool.h
@@ -628,6 +628,7 @@ extern void pool_shmem_exit(int code);
 
 extern void pool_semaphore_create(int numSems);
 extern void pool_semaphore_lock(int semNum);
+extern int     pool_semaphore_lock_allow_interrupt(int semNum);
 extern void pool_semaphore_unlock(int semNum);
 
 extern BackendInfo *pool_get_node_info(int node_number);

diff --git a/src/protocol/child.c b/src/protocol/child.c
index 096ffd40742477ed2c9ec7d52ce487f64e9be776..8c143e24b96a62cadce22425550f5517f1e987b3 100644 (file)
--- a/src/protocol/child.c
+++ b/src/protocol/child.c
@@ -1981,7 +1981,50 @@ wait_for_new_connections(int *fds, struct timeval *timeout, SockAddr *saddr)
         */
        if (SERIALIZE_ACCEPT)
        {
-               pool_semaphore_lock(ACCEPT_FD_SEM);
+               if (pool_config->connection_life_time == 0)
+               {
+                       pool_semaphore_lock(ACCEPT_FD_SEM);
+               }
+               else
+               {
+                       int sts;
+
+                       for (;;)
+                       {
+                               sts = pool_semaphore_lock_allow_interrupt(ACCEPT_FD_SEM);
+
+                               /* Interrupted by alarm */
+                               if (sts == -2)
+                               {
+                                       /*
+                                        * Check if there are expired connection_life_time.
+                                        */
+                                       if (backend_timer_expired)
+                                       {
+                                               /*
+                                                * We add 10 seconds to connection_life_time so that there's
+                                                * enough margin.
+                                                */
+                                               int     seconds = pool_config->connection_life_time + 10;
+
+                                               while (seconds-- > 0)
+                                               {
+                                                       /* check backend timer is expired */
+                                                       if (backend_timer_expired)
+                                                       {
+                                                               pool_backend_timer();
+                                                               backend_timer_expired = 0;
+                                                               break;
+                                                       }
+                                                       sleep(1);
+                                               }
+                                       }
+                               }
+                               else    /* success or other error */
+                                       break;
+                       }
+               }
+
                set_ps_display("wait for connection request", false);
                ereport(DEBUG1,
                           (errmsg("LOCKING select()")));

diff --git a/src/protocol/pool_connection_pool.c b/src/protocol/pool_connection_pool.c
index fa93eb96e703f7180bc1f45927add53c24eb7276..979f2fcd08f26d72955f9f612f70c6160f608ccd 100644 (file)
--- a/src/protocol/pool_connection_pool.c
+++ b/src/protocol/pool_connection_pool.c
@@ -384,7 +384,7 @@ void pool_backend_timer(void)
        now = time(NULL);
 
        ereport(DEBUG1,
-               (errmsg("backend timer handler called at%ld", now)));
+                       (errmsg("backend timer handler called at %ld", now)));
 
        for (i=0;i<pool_config->max_pool;i++, p++)
        {

diff --git a/src/utils/pool_sema.c b/src/utils/pool_sema.c
index abe4b55a4858cce18314e6e3f7a961b5e1600756..befc63857e1b7b42c203887e4be29ee62dd20e6a 100644 (file)
--- a/src/utils/pool_sema.c
+++ b/src/utils/pool_sema.c
@@ -129,6 +129,48 @@ pool_semaphore_lock(int semNum)
                        (errmsg("failed to lock semaphore error:\"%s\"",strerror(errno))));
 }
 
+/*
+ * Lock a semaphore (decrement count), blocking if count would be < 0.
+ * Unlike pool_semaphore_lock, this returns if interrupted.
+ * Return values:
+ * 0: succeeded in acquiring lock.
+ * -1: error.
+ * -2: interrupted.
+ */
+int
+pool_semaphore_lock_allow_interrupt(int semNum)
+{
+       int                     errStatus;
+       struct sembuf sops;
+
+       sops.sem_op = -1;                       /* decrement */
+       sops.sem_flg = SEM_UNDO;
+       sops.sem_num = semNum;
+
+       /*
+        * Note: if errStatus is -1 and errno == EINTR then it means we returned
+        * from the operation prematurely because we were sent a signal.
+        */
+       errStatus = semop(semId, &sops, 1);
+
+       if (errStatus < 0)
+       {
+               if (errno == EINTR)
+               {
+                       ereport(DEBUG1,
+                                       (errmsg("interrupted while trying to lock semaphore")));
+                       return -2;
+               }
+               else
+               {
+                       ereport(WARNING,
+                                       (errmsg("failed to lock semaphore error:\"%s\"", strerror(errno))));
+                       return -1;
+               }
+       }
+       return 0;
+}
+
 /*
  * Unlock a semaphore (increment count)
  */