On Fri, Feb 10, 2023 at 10:51:20AM -0800, Nathan Bossart wrote:
> On Fri, Feb 10, 2023 at 10:18:34AM -0500, Tom Lane wrote:
>> Robert Haas <robertmh...@gmail.com> writes:
>>> I wonder if we should have a wrapper around WaitLatch() that documents
>>> that if the latch is set before the time expires, it will reset the
>>> latch and try again to wait for the remaining time, after checking for
>>> interrupts etc.
>> 
>> Resetting the latch seems not very friendly for general-purpose use
>> ... although I guess we could set it again on the way out.
>> 
>> BTW, we have an existing pg_sleep() function that deals with all
>> of this except re-setting the latch.
> 
> That seems workable.  I think it might also need to accept a function
> pointer for custom interrupt checking (e.g., archiver code should use
> HandlePgArchInterrupts()).  I'll put something together if that sounds
> alright.

Here is a work-in-progress patch.  I quickly scanned through my previous
patch and applied this new function everywhere it seemed safe to use (which
unfortunately is rather limited).

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 3feee28d19..d78ae26b78 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -2976,7 +2976,7 @@ _bt_pendingfsm_finalize(Relation rel, BTVacState *vstate)
 	 * never be effective without some other backend concurrently consuming an
 	 * XID.
 	 */
-	pg_usleep(5000000L);
+	pg_sleep(5, NULL);
 #endif
 
 	/*
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index ff6149a179..c1286e27c2 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -687,7 +687,7 @@ AutoVacLauncherMain(int argc, char *argv[])
 				 * of a worker will continue to fail in the same way.
 				 */
 				AutoVacuumShmem->av_signal[AutoVacForkFailed] = false;
-				pg_usleep(1000000L);	/* 1s */
+				pg_sleep(1, HandleAutoVacLauncherInterrupts);
 				SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_WORKER);
 				continue;
 			}
@@ -1708,7 +1708,7 @@ AutoVacWorkerMain(int argc, char *argv[])
 				(errmsg_internal("autovacuum: processing database \"%s\"", dbname)));
 
 		if (PostAuthDelay)
-			pg_usleep(PostAuthDelay * 1000000L);
+			pg_sleep(PostAuthDelay, NULL);
 
 		/* And do an appropriate amount of work */
 		recentXid = ReadNextTransactionId();
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e551af2905..20a0f05399 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -450,7 +450,7 @@ pgarch_ArchiverCopyLoop(void)
 				}
 
 				/* wait a bit before retrying */
-				pg_usleep(1000000L);
+				pg_sleep(1, HandlePgArchInterrupts);
 				continue;
 			}
 
@@ -482,7 +482,7 @@ pgarch_ArchiverCopyLoop(void)
 									xlog)));
 					return;		/* give up archiving for now */
 				}
-				pg_usleep(1000000L);	/* wait a bit before retrying */
+				pg_sleep(1, HandlePgArchInterrupts);	/* wait a bit before retrying */
 			}
 		}
 	}
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 220ddb8c01..29a9daca8e 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -365,12 +365,18 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
 
 /*
  * pg_sleep - delay for N seconds
+ *
+ * If the caller provides a NULL 'check_interrupts' function,
+ * CHECK_FOR_INTERRUPTS() is used instead.
+ *
+ * This function is careful to set the latch before returning if it had to be
+ * reset.
  */
-Datum
-pg_sleep(PG_FUNCTION_ARGS)
+void
+pg_sleep(float8 secs, void (*check_interrupts) (void))
 {
-	float8		secs = PG_GETARG_FLOAT8(0);
 	float8		endtime;
+	bool		latch_set = false;
 
 	/*
 	 * We sleep using WaitLatch, to ensure that we'll wake up promptly if an
@@ -392,8 +398,12 @@ pg_sleep(PG_FUNCTION_ARGS)
 	{
 		float8		delay;
 		long		delay_ms;
+		int			rc;
 
-		CHECK_FOR_INTERRUPTS();
+		if (check_interrupts == NULL)
+			CHECK_FOR_INTERRUPTS();
+		else
+			(*check_interrupts) ();
 
 		delay = endtime - GetNowFloat();
 		if (delay >= 600.0)
@@ -403,13 +413,30 @@ pg_sleep(PG_FUNCTION_ARGS)
 		else
 			break;
 
-		(void) WaitLatch(MyLatch,
-						 WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
-						 delay_ms,
-						 WAIT_EVENT_PG_SLEEP);
-		ResetLatch(MyLatch);
+		rc = WaitLatch(MyLatch,
+					   WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+					   delay_ms,
+					   WAIT_EVENT_PG_SLEEP);
+
+		if (rc & WL_LATCH_SET)
+		{
+			latch_set = true;
+			ResetLatch(MyLatch);
+		}
 	}
 
+	if (latch_set)
+		SetLatch(MyLatch);
+}
+
+/*
+ * pg_sleep_sql - SQL-callable version of pg_sleep()
+ */
+Datum
+pg_sleep_sql(PG_FUNCTION_ARGS)
+{
+	pg_sleep(PG_GETARG_FLOAT8(0), NULL);
+	ResetLatch(MyLatch);	/* pg_sleep might've set latch before returning */
 	PG_RETURN_VOID();
 }
 
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index c0f2a8a77c..b0d6bd58c7 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6529,7 +6529,7 @@
   prosrc => 'pg_ls_dir' },
 { oid => '2626', descr => 'sleep for the specified time in seconds',
   proname => 'pg_sleep', provolatile => 'v', prorettype => 'void',
-  proargtypes => 'float8', prosrc => 'pg_sleep' },
+  proargtypes => 'float8', prosrc => 'pg_sleep_sql' },
 { oid => '3935', descr => 'sleep for the specified interval',
   proname => 'pg_sleep_for', prolang => 'sql', provolatile => 'v',
   prorettype => 'void', proargtypes => 'interval',
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index c309e0233d..3c1fb84c53 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -495,4 +495,7 @@ extern void RestoreClientConnectionInfo(char *conninfo);
 /* in executor/nodeHash.c */
 extern size_t get_hash_memory_limit(void);
 
+/* in utils/adt/misc.c */
+extern void pg_sleep(float8 secs, void (*check_interrupts) (void));
+
 #endif							/* MISCADMIN_H */

Reply via email to