Fix connection_life_time not working when serialize_accept is enabled.
authorTatsuo Ishii <ishii@sraoss.co.jp>
Tue, 1 Sep 2020 03:40:18 +0000 (12:40 +0900)
committerTatsuo Ishii <ishii@sraoss.co.jp>
Tue, 1 Sep 2020 03:45:20 +0000 (12:45 +0900)
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

src/include/pool.h
src/protocol/child.c
src/protocol/pool_connection_pool.c
src/utils/pool_sema.c

index 240d1507b30925d08763b54b87133a53ad8040dd..96a2ba43be94f866144f84a7be38ecd99ea7f9d2 100644 (file)
@@ -643,6 +643,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);
index bffd4a8fd71b2e9b10437548bccb47982f87313d..c64b1806f79415f59f130cb1c4e1db19ecb46447 100644 (file)
@@ -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()")));
index 69abc220a51787404daa5dba7966f5d3f06d51f5..951d73f33d3ac039deafdb8614fc92294726f0e7 100644 (file)
@@ -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++)
        {
index abe4b55a4858cce18314e6e3f7a961b5e1600756..befc63857e1b7b42c203887e4be29ee62dd20e6a 100644 (file)
@@ -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)
  */