pgpool2.git / summary / log / commit / refs

commit    f8b63be8862cd54c0f3a9b13da9602928aaf330d
Author:   Tatsuo Ishii <ishii@sraoss.co.jp>
Date:     Tue Sep 01 03:21:50 2020 +0000

    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


 src/include/utils/pool_ipc.h        |  1 +
 src/protocol/child.c                | 45 ++++++++++++++++++++++++++++++++++++-
 src/protocol/pool_connection_pool.c |  2 +-
 src/utils/pool_sema.c               | 42 ++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/src/include/utils/pool_ipc.h b/src/include/utils/pool_ipc.h
index ba3f9ed26..45cd5f8ca 100644
--- 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 976aa0cd4..4b2bd43e7 100644
--- 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 059543356..cb5026713 100644
--- 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 16069fbe6..9558c7fd7 100644
--- 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)
  */


[parent: 9c8ea012051e]