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/utils/pool_ipc.h b/src/include/utils/pool_ipc.h
index ba3f9ed26cdcfab5aa29099c665f0c9d3b1db51f..45cd5f8ca6adb0b19d8d22f3f671faad28834052 100644 (file)
--- a/src/include/utils/pool_ipc.h
+++ b/src/include/utils/pool_ipc.h
@@ -32,6 +32,7 @@ extern void on_exit_reset(void);
 /*pool_sema.c*/
 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);
 
 #endif                                                 /* IPC_H */

diff --git a/src/protocol/child.c b/src/protocol/child.c
index 976aa0cd44448cd6a5b326d9e2ccb014b45f6e04..4b2bd43e7058a6ebdd28b353dc67fb7ae0ba28b9 100644 (file)
--- a/src/protocol/child.c
+++ b/src/protocol/child.c
@@ -1496,7 +1496,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 05954335602f8daf633e07904aa27f9c396687cb..cb50267138f30327e932d155b9ea352905e1a744 100644 (file)
--- a/src/protocol/pool_connection_pool.c
+++ b/src/protocol/pool_connection_pool.c
@@ -405,7 +405,7 @@ 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 16069fbe6a90872187b920eccfb8997ad15f8bef..9558c7fd705617fef06b15a03c1285301e4034b3 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)
  */