On Mon, Mar 13, 2023 at 02:16:31PM -0700, Nathan Bossart wrote:
> At the moment, I'm thinking about either removing the check_interrupts
> function pointer argument altogether or making it optional for code paths
> where we might not want any interrupt handling to run.  In the former
> approach, we could simply call WaitLatch() without a latch.  While this
> wouldn't do any interrupt handling, we'd still get custom wait event
> support and better responsiveness when the postmaster dies, which is
> strictly better than what's there today.  And many of these sleeps are in
> uncommon or debug paths, so delaying interrupt handling might be okay.  In
> the latter approach, we would have interrupt handling, but I'm worried that
> would be easy to get wrong.  I think it would be nice to have interrupt
> handling if possible, so I'm still (over)thinking about this.

I started on the former approach (work-in-progress patch attached), but I
figured I'd ask whether folks are alright with this before I spend more
time on it.  Many of the sleeps in question are relatively short, are
intended for debugging, or are meant to prevent errors from repeating as
fast as possible, so I don't know if we should bother adding interrupt
handling support.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 7ff61d6d1a4829e861a57bae621fb939f0133bc7 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Wed, 26 Jul 2023 16:24:01 -0700
Subject: [PATCH v3 1/1] WORK IN PROGRESS: pg_msleep

---
 src/backend/access/nbtree/nbtpage.c             |  2 +-
 src/backend/access/transam/multixact.c          |  3 ++-
 src/backend/access/transam/xlog.c               |  6 +++---
 src/backend/access/transam/xlogutils.c          |  3 ++-
 src/backend/commands/vacuum.c                   |  4 +---
 src/backend/libpq/pqcomm.c                      |  3 ++-
 src/backend/port/win32/socket.c                 |  2 +-
 src/backend/postmaster/autovacuum.c             |  8 ++++----
 src/backend/postmaster/bgworker.c               |  2 +-
 src/backend/postmaster/bgwriter.c               |  2 +-
 src/backend/postmaster/checkpointer.c           |  4 ++--
 src/backend/postmaster/pgarch.c                 |  6 ++++--
 src/backend/postmaster/postmaster.c             |  2 +-
 src/backend/postmaster/walwriter.c              |  2 +-
 src/backend/replication/walsender.c             |  2 +-
 src/backend/storage/file/fd.c                   |  4 ++--
 src/backend/storage/ipc/latch.c                 | 14 ++++++++++++++
 src/backend/storage/ipc/procarray.c             |  2 +-
 src/backend/storage/ipc/standby.c               |  4 ++--
 src/backend/storage/lmgr/lmgr.c                 |  4 ++--
 src/backend/utils/activity/wait_event_names.txt |  2 ++
 src/backend/utils/init/postinit.c               |  4 ++--
 src/include/storage/latch.h                     |  1 +
 23 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index d78971bfe8..ab31da93a4 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -3013,7 +3013,7 @@ _bt_pendingfsm_finalize(Relation rel, BTVacState *vstate)
 	 * never be effective without some other backend concurrently consuming an
 	 * XID.
 	 */
-	pg_usleep(5000000L);
+	pg_msleep(5000, WAIT_EVENT_PG_SLEEP);
 #endif
 
 	/*
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index abb022e067..18c4041b19 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -90,6 +90,7 @@
 #include "utils/builtins.h"
 #include "utils/memutils.h"
 #include "utils/snapmgr.h"
+#include "utils/wait_event.h"
 
 
 /*
@@ -1390,7 +1391,7 @@ retry:
 			/* Corner case 2: next multixact is still being filled in */
 			LWLockRelease(MultiXactOffsetSLRULock);
 			CHECK_FOR_INTERRUPTS();
-			pg_usleep(1000L);
+			pg_msleep(1, WAIT_EVENT_PG_SLEEP);
 			goto retry;
 		}
 
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 60c0b7ec3a..0dce93637b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5130,7 +5130,7 @@ StartupXLOG(void)
 	/* This is just to allow attaching to startup process with a debugger */
 #ifdef XLOG_REPLAY_DELAY
 	if (ControlFile->state != DB_SHUTDOWNED)
-		pg_usleep(60000000L);
+		pg_msleep(60000, WAIT_EVENT_PG_SLEEP);
 #endif
 
 	/*
@@ -6710,7 +6710,7 @@ CreateCheckPoint(int flags)
 	{
 		do
 		{
-			pg_usleep(10000L);	/* wait for 10 msec */
+			pg_msleep(10, WAIT_EVENT_PG_SLEEP);
 		} while (HaveVirtualXIDsDelayingChkpt(vxids, nvxids,
 											  DELAY_CHKPT_START));
 	}
@@ -6723,7 +6723,7 @@ CreateCheckPoint(int flags)
 	{
 		do
 		{
-			pg_usleep(10000L);	/* wait for 10 msec */
+			pg_msleep(10, WAIT_EVENT_PG_SLEEP);
 		} while (HaveVirtualXIDsDelayingChkpt(vxids, nvxids,
 											  DELAY_CHKPT_COMPLETE));
 	}
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index e174a2a891..ff24a1f0b8 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -27,6 +27,7 @@
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "storage/fd.h"
+#include "storage/latch.h"
 #include "storage/smgr.h"
 #include "utils/guc.h"
 #include "utils/hsearch.h"
@@ -958,7 +959,7 @@ read_local_xlog_page_guts(XLogReaderState *state, XLogRecPtr targetPagePtr,
 			}
 
 			CHECK_FOR_INTERRUPTS();
-			pg_usleep(1000L);
+			pg_msleep(1, WAIT_EVENT_PG_SLEEP);
 		}
 		else
 		{
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 69ac276687..f82f6e8878 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -2384,9 +2384,7 @@ vacuum_delay_point(void)
 		if (msec > vacuum_cost_delay * 4)
 			msec = vacuum_cost_delay * 4;
 
-		pgstat_report_wait_start(WAIT_EVENT_VACUUM_DELAY);
-		pg_usleep(msec * 1000);
-		pgstat_report_wait_end();
+		pg_msleep(msec, WAIT_EVENT_VACUUM_DELAY);
 
 		/*
 		 * We don't want to ignore postmaster death during very long vacuums
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index da5bb5fc5d..e1c620fd26 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -80,6 +80,7 @@
 #include "storage/ipc.h"
 #include "utils/guc_hooks.h"
 #include "utils/memutils.h"
+#include "utils/wait_event.h"
 
 /*
  * Cope with the various platform-specific ways to spell TCP keepalive socket
@@ -714,7 +715,7 @@ StreamConnection(pgsocket server_fd, Port *port)
 		 * (The most likely reason for failure is being out of kernel file
 		 * table slots; we can do little except hope some will get freed up.)
 		 */
-		pg_usleep(100000L);		/* wait 0.1 sec */
+		pg_msleep(100, WAIT_EVENT_PG_SLEEP);
 		return STATUS_ERROR;
 	}
 
diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c
index 9c339397d1..c328a45e45 100644
--- a/src/backend/port/win32/socket.c
+++ b/src/backend/port/win32/socket.c
@@ -436,7 +436,7 @@ pgwin32_recv(SOCKET s, char *buf, int len, int f)
 		 * again.  We try up to 5 times - if it fails more than that it's not
 		 * likely to ever come back.
 		 */
-		pg_usleep(10000);
+		pg_msleep(10, WAIT_EVENT_PG_SLEEP);
 	}
 	ereport(NOTICE,
 			(errmsg_internal("could not read from ready socket (after retries)")));
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index ae9be9b911..1c49368521 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -453,7 +453,7 @@ AutoVacLauncherMain(int argc, char *argv[])
 			(errmsg_internal("autovacuum launcher started")));
 
 	if (PostAuthDelay)
-		pg_usleep(PostAuthDelay * 1000000L);
+		pg_msleep(PostAuthDelay * 1000, WAIT_EVENT_POST_AUTH_DELAY);
 
 	SetProcessingMode(InitProcessing);
 
@@ -572,7 +572,7 @@ AutoVacLauncherMain(int argc, char *argv[])
 		 * Sleep at least 1 second after any error.  We don't want to be
 		 * filling the error logs as fast as we can.
 		 */
-		pg_usleep(1000000L);
+		pg_msleep(1000, WAIT_EVENT_PG_SLEEP);
 	}
 
 	/* We can now handle ereport(ERROR) */
@@ -698,7 +698,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_msleep(1000, WAIT_EVENT_PG_SLEEP);
 				SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_WORKER);
 				continue;
 			}
@@ -1714,7 +1714,7 @@ AutoVacWorkerMain(int argc, char *argv[])
 				(errmsg_internal("autovacuum: processing database \"%s\"", dbname)));
 
 		if (PostAuthDelay)
-			pg_usleep(PostAuthDelay * 1000000L);
+			pg_msleep(PostAuthDelay * 1000, WAIT_EVENT_POST_AUTH_DELAY);
 
 		/* And do an appropriate amount of work */
 		recentXid = ReadNextTransactionId();
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 5b4bd71694..0cf06539a7 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -759,7 +759,7 @@ StartBackgroundWorker(void)
 
 	/* Apply PostAuthDelay */
 	if (PostAuthDelay > 0)
-		pg_usleep(PostAuthDelay * 1000000L);
+		pg_msleep(PostAuthDelay * 1000, WAIT_EVENT_POST_AUTH_DELAY);
 
 	/*
 	 * Set up signal handlers.
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index caad642ec9..3f683286fe 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -195,7 +195,7 @@ BackgroundWriterMain(void)
 		 * to be repeated, and we don't want to be filling the error logs as
 		 * fast as we can.
 		 */
-		pg_usleep(1000000L);
+		pg_msleep(1000, WAIT_EVENT_PG_SLEEP);
 
 		/*
 		 * Close all open files after any error.  This is helpful on Windows,
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index ace9893d95..a79bca7a11 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -309,7 +309,7 @@ CheckpointerMain(void)
 		 * to be repeated, and we don't want to be filling the error logs as
 		 * fast as we can.
 		 */
-		pg_usleep(1000000L);
+		pg_msleep(1000, WAIT_EVENT_CHECKPOINTER_MAIN);
 
 		/*
 		 * Close all open files after any error.  This is helpful on Windows,
@@ -1005,7 +1005,7 @@ RequestCheckpoint(int flags)
 			break;				/* signal sent successfully */
 
 		CHECK_FOR_INTERRUPTS();
-		pg_usleep(100000L);		/* wait 0.1 sec, then retry */
+		pg_msleep(100, WAIT_EVENT_CHECKPOINTER_MAIN);	/* wait 0.1 sec, then retry */
 	}
 
 	/*
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 46af349564..f526f51ee9 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -453,7 +453,7 @@ pgarch_ArchiverCopyLoop(void)
 				}
 
 				/* wait a bit before retrying */
-				pg_usleep(1000000L);
+				pg_msleep(1000, WAIT_EVENT_ARCHIVER_MAIN);
 				continue;
 			}
 
@@ -485,7 +485,9 @@ pgarch_ArchiverCopyLoop(void)
 									xlog)));
 					return;		/* give up archiving for now */
 				}
-				pg_usleep(1000000L);	/* wait a bit before retrying */
+
+				/* wait a bit before retrying */
+				pg_msleep(1000, WAIT_EVENT_ARCHIVER_MAIN);
 			}
 		}
 	}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 9c8ec779f9..52b8a37259 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4264,7 +4264,7 @@ BackendInitialize(Port *port)
 	 * is not honored until after authentication.)
 	 */
 	if (PreAuthDelay > 0)
-		pg_usleep(PreAuthDelay * 1000000L);
+		pg_msleep(PreAuthDelay * 1000, WAIT_EVENT_PRE_AUTH_DELAY);
 
 	/* This flag will remain set until InitPostgres finishes authentication */
 	ClientAuthInProgress = true;	/* limit visibility of log messages */
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 266fbc2339..d84ebb30b4 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -188,7 +188,7 @@ WalWriterMain(void)
 		 * to be repeated, and we don't want to be filling the error logs as
 		 * fast as we can.
 		 */
-		pg_usleep(1000000L);
+		pg_msleep(1000, WAIT_EVENT_WAL_WRITER_MAIN);
 
 		/*
 		 * Close all open files after any error.  This is helpful on Windows,
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index d27ef2985d..63626187cb 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3461,7 +3461,7 @@ WalSndWaitStopping(void)
 		if (all_stopped)
 			return;
 
-		pg_usleep(10000L);		/* wait for 10 msec */
+		pg_msleep(10, WAIT_EVENT_PG_SLEEP);
 	}
 }
 
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index a027a8aabc..4f76d7ec19 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -2124,7 +2124,7 @@ retry:
 		switch (error)
 		{
 			case ERROR_NO_SYSTEM_RESOURCES:
-				pg_usleep(1000L);
+				pg_msleep(1, WAIT_EVENT_PG_SLEEP);
 				errno = EINTR;
 				break;
 			default:
@@ -2222,7 +2222,7 @@ retry:
 		switch (error)
 		{
 			case ERROR_NO_SYSTEM_RESOURCES:
-				pg_usleep(1000L);
+				pg_msleep(1, WAIT_EVENT_PG_SLEEP);
 				errno = EINTR;
 				break;
 			default:
diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c
index db889385b7..78d023c2fd 100644
--- a/src/backend/storage/ipc/latch.c
+++ b/src/backend/storage/ipc/latch.c
@@ -2264,3 +2264,17 @@ drain(void)
 }
 
 #endif
+
+/*
+ * Convenient wrapper around WaitLatch(NULL, ...).
+ */
+void
+pg_msleep(int timeout, uint32 wait_event_info)
+{
+	Assert(timeout >= 0);
+
+	(void) WaitLatch(NULL,
+					 WL_EXIT_ON_PM_DEATH | WL_TIMEOUT,
+					 timeout,
+					 wait_event_info);
+}
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 2a3da49b8f..f3740db9ab 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -3745,7 +3745,7 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared)
 			(void) kill(autovac_pids[index], SIGTERM);	/* ignore any error */
 
 		/* sleep, then try again */
-		pg_usleep(100 * 1000L); /* 100ms */
+		pg_msleep(100, WAIT_EVENT_PG_SLEEP);
 	}
 
 	return true;				/* timed out, still conflicts */
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index cc22d2e87c..f2035a9cf4 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -397,7 +397,7 @@ ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 				 * an unresponsive backend when system is heavily loaded.
 				 */
 				if (pid != 0)
-					pg_usleep(5000L);
+					pg_msleep(5, WAIT_EVENT_PG_SLEEP);
 			}
 
 			if (waitStart != 0 && (!logged_recovery_conflict || !waiting))
@@ -587,7 +587,7 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
 		 * Wait awhile for them to die so that we avoid flooding an
 		 * unresponsive backend when system is heavily loaded.
 		 */
-		pg_usleep(10000);
+		pg_msleep(10, WAIT_EVENT_PG_SLEEP);
 	}
 }
 
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index ee9b89a672..65955d70bf 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -722,7 +722,7 @@ XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
 		 * through, to avoid slowing down the normal case.)
 		 */
 		if (!first)
-			pg_usleep(1000L);
+			pg_msleep(1, WAIT_EVENT_PG_SLEEP);
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
 	}
@@ -760,7 +760,7 @@ ConditionalXactLockTableWait(TransactionId xid)
 
 		/* See XactLockTableWait about this case */
 		if (!first)
-			pg_usleep(1000L);
+			pg_msleep(1, WAIT_EVENT_PG_SLEEP);
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
 	}
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index 3fabad96d9..2c6b527a72 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -154,6 +154,8 @@ Section: ClassName - WaitEventTimeout
 WAIT_EVENT_BASE_BACKUP_THROTTLE	BaseBackupThrottle	"Waiting during base backup when throttling activity."
 WAIT_EVENT_CHECKPOINT_WRITE_DELAY	CheckpointWriteDelay	"Waiting between writes while performing a checkpoint."
 WAIT_EVENT_PG_SLEEP	PgSleep	"Waiting due to a call to <function>pg_sleep</function> or a sibling function."
+WAIT_EVENT_PRE_AUTH_DELAY	PreAuthDelay	"Waiting before authentication."
+WAIT_EVENT_POST_AUTH_DELAY	PostAuthDelay	"Waiting after authentication."
 WAIT_EVENT_RECOVERY_APPLY_DELAY	RecoveryApplyDelay	"Waiting to apply WAL during recovery because of a delay setting."
 WAIT_EVENT_RECOVERY_RETRIEVE_RETRY_INTERVAL	RecoveryRetrieveRetryInterval	"Waiting during recovery when WAL data is not available from any source (<filename>pg_wal</filename>, archive or stream)."
 WAIT_EVENT_REGISTER_SYNC_REQUEST	RegisterSyncRequest	"Waiting while sending synchronization requests to the checkpointer, because the request queue is full."
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index f31b85c013..8846a61aff 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -983,7 +983,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
 
 		/* Apply PostAuthDelay as soon as we've read all options */
 		if (PostAuthDelay > 0)
-			pg_usleep(PostAuthDelay * 1000000L);
+			pg_msleep(PostAuthDelay * 1000, WAIT_EVENT_POST_AUTH_DELAY);
 
 		/* initialize client encoding */
 		InitializeClientEncoding();
@@ -1200,7 +1200,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
 
 	/* Apply PostAuthDelay as soon as we've read all options */
 	if (PostAuthDelay > 0)
-		pg_usleep(PostAuthDelay * 1000000L);
+		pg_msleep(PostAuthDelay * 1000, WAIT_EVENT_POST_AUTH_DELAY);
 
 	/*
 	 * Initialize various default states that can't be set up until we've
diff --git a/src/include/storage/latch.h b/src/include/storage/latch.h
index 99cc47874a..2b92d75e1f 100644
--- a/src/include/storage/latch.h
+++ b/src/include/storage/latch.h
@@ -190,5 +190,6 @@ extern int	WaitLatchOrSocket(Latch *latch, int wakeEvents,
 extern void InitializeLatchWaitSet(void);
 extern int	GetNumRegisteredWaitEvents(WaitEventSet *set);
 extern bool WaitEventSetCanReportClosed(void);
+extern void pg_msleep(int timeout, uint32 wait_event_info);
 
 #endif							/* LATCH_H */
-- 
2.25.1

Reply via email to