pg_usleep for multisecond delays
I just found myself carefully counting the zeros in a call to pg_usleep().
Besides getting my eyes checked, perhaps there should be a wrapper called
pg_ssleep() than can be used for multisecond sleeps. Or maybe the
USECS_PER_SEC macro should be used more widely. I attached a patch for the
former approach. I don't have a strong opinion, but I do think it's worth
improving readability a bit here.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
pg_ssleep.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 3feee28d19..63de896cae 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_ssleep(5);
#endif
/*
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f9f0f6db8d..87664045d0 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5103,7 +5103,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_ssleep(60);
#endif
/*
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index ff6149a179..744adff984 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -441,7 +441,7 @@ AutoVacLauncherMain(int argc, char *argv[])
(errmsg_internal("autovacuum launcher started")));
if (PostAuthDelay)
- pg_usleep(PostAuthDelay * 1000000L);
+ pg_ssleep(PostAuthDelay);
SetProcessingMode(InitProcessing);
@@ -561,7 +561,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_ssleep(1);
}
/* We can now handle ereport(ERROR) */
@@ -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_ssleep(1);
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_ssleep(PostAuthDelay);
/* And do an appropriate amount of work */
recentXid = ReadNextTransactionId();
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 0dd22b2351..6d38dfeeba 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_ssleep(PostAuthDelay);
/*
* Set up signal handlers.
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 9bb47da404..147f9b1e38 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -196,7 +196,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_ssleep(1);
/*
* 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 aaad5c5228..12786229dd 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -310,7 +310,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_ssleep(1);
/*
* Close all open files after any error. This is helpful on Windows,
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e551af2905..6460c69726 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_ssleep(1);
continue;
}
@@ -482,7 +482,7 @@ pgarch_ArchiverCopyLoop(void)
xlog)));
return; /* give up archiving for now */
}
- pg_usleep(1000000L); /* wait a bit before retrying */
+ pg_ssleep(1); /* wait a bit before retrying */
}
}
}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 2552327d90..6b80f423a8 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4292,7 +4292,7 @@ BackendInitialize(Port *port)
* is not honored until after authentication.)
*/
if (PreAuthDelay > 0)
- pg_usleep(PreAuthDelay * 1000000L);
+ pg_ssleep(PreAuthDelay);
/* 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 513e580c51..a003717efd 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -189,7 +189,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_ssleep(1);
/*
* Close all open files after any error. This is helpful on Windows,
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 2f07ca7a0e..5647795f4a 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -973,7 +973,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_ssleep(PostAuthDelay);
/* initialize client encoding */
InitializeClientEncoding();
@@ -1180,7 +1180,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_ssleep(PostAuthDelay);
/*
* Initialize various default states that can't be set up until we've
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index f3c7937a1d..ca8bfac936 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -991,7 +991,7 @@ main(int argc, char **argv)
/* translator: check source for value for %d */
pg_log_info("disconnected; waiting %d seconds to try again",
RECONNECT_SLEEP_TIME);
- pg_usleep(RECONNECT_SLEEP_TIME * 1000000);
+ pg_ssleep(RECONNECT_SLEEP_TIME);
}
}
}
diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index 5b2edebe41..a1fc352eb8 100644
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -151,7 +151,7 @@ exec_prog(const char *log_filename, const char *opt_log_file,
for (iter = 0; iter < 4 && log == NULL; iter++)
{
- pg_usleep(1000000); /* 1 sec */
+ pg_ssleep(1);
log = fopen(log_file, "a");
}
}
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 44b5c8726e..610ebc777c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1217,7 +1217,7 @@ main(int argc, char **argv)
break;
else
{
- pg_usleep(1000000L); /* 1 second */
+ pg_ssleep(1);
continue;
}
}
diff --git a/src/include/port.h b/src/include/port.h
index e66193bed9..43ce931766 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -163,6 +163,11 @@ extern int pg_disable_aslr(void);
/* Portable delay handling */
extern void pg_usleep(long microsec);
+static inline void pg_ssleep(long sec)
+{
+ pg_usleep(sec * 1000000L);
+}
+
/* Portable SQL-like case-independent comparisons and conversions */
extern int pg_strcasecmp(const char *s1, const char *s2);
extern int pg_strncasecmp(const char *s1, const char *s2, size_t n);
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 6cd5998b9d..018237d750 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -2386,7 +2386,7 @@ regression_main(int argc, char *argv[],
exit(2);
}
- pg_usleep(1000000L);
+ pg_ssleep(1);
}
if (i >= wait_seconds)
{
Hi,
On 2023-02-09 12:59:29 -0800, Nathan Bossart wrote:
I just found myself carefully counting the zeros in a call to pg_usleep().
Besides getting my eyes checked, perhaps there should be a wrapper called
pg_ssleep() than can be used for multisecond sleeps. Or maybe the
USECS_PER_SEC macro should be used more widely. I attached a patch for the
former approach. I don't have a strong opinion, but I do think it's worth
improving readability a bit here.
pg_usleep() should pretty much never used for sleeps that long, at least in
the backend - depending on the platform they're not interruptible. Most of the
things changed here are debugging tools, but even so, it's e.g. pretty
annoying. E.g. you can't normally shut down while a backend is in
pre_auth_delay.
So I'm not sure it's the right direction to make pg_usleep() easier to use...
Greetings,
Andres Freund
On Thu, Feb 09, 2023 at 01:30:27PM -0800, Andres Freund wrote:
On 2023-02-09 12:59:29 -0800, Nathan Bossart wrote:
I just found myself carefully counting the zeros in a call to pg_usleep().
Besides getting my eyes checked, perhaps there should be a wrapper called
pg_ssleep() than can be used for multisecond sleeps. Or maybe the
USECS_PER_SEC macro should be used more widely. I attached a patch for the
former approach. I don't have a strong opinion, but I do think it's worth
improving readability a bit here.pg_usleep() should pretty much never used for sleeps that long, at least in
the backend - depending on the platform they're not interruptible. Most of the
things changed here are debugging tools, but even so, it's e.g. pretty
annoying. E.g. you can't normally shut down while a backend is in
pre_auth_delay.So I'm not sure it's the right direction to make pg_usleep() easier to use...
Hm... We might be able to use WaitLatch() for some of these.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
At Thu, 9 Feb 2023 14:51:14 -0800, Nathan Bossart <nathandbossart@gmail.com> wrote in
On Thu, Feb 09, 2023 at 01:30:27PM -0800, Andres Freund wrote:
So I'm not sure it's the right direction to make pg_usleep() easier to use...
Hm... We might be able to use WaitLatch() for some of these.
And I think we are actully going to reduce or eliminate the use of
pg_sleep().
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Thu, Feb 09, 2023 at 02:51:14PM -0800, Nathan Bossart wrote:
Hm... We might be able to use WaitLatch() for some of these.
Perhaps less than you think as a bit of work has been done in the last
few years to reduce the gap and make such code paths more responsive,
still I would not be surprised to find a couple of these..
I would let the portions of the code related to things like
pre_auth_delay or XLOG_REPLAY_DELAY as they are, though, without an
extra pg_Xsleep() implementation.
--
Michael
On 2023-Feb-09, Nathan Bossart wrote:
I just found myself carefully counting the zeros in a call to pg_usleep().
Besides getting my eyes checked, perhaps there should be a wrapper called
pg_ssleep() than can be used for multisecond sleeps. Or maybe the
USECS_PER_SEC macro should be used more widely. I attached a patch for the
former approach. I don't have a strong opinion, but I do think it's worth
improving readability a bit here.
Hmm, it seems about half the patch would go away if you were to add a
PostAuthDelaySleep() function.
@@ -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_ssleep(5);
Maybe for these cases where a WaitLatch is not desired, it'd be simpler
to do pg_usleep (5L * 1000 * 1000);
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"I am amazed at [the pgsql-sql] mailing list for the wonderful support, and
lack of hesitasion in answering a lost soul's question, I just wished the rest
of the mailing list could be like this." (Fotis)
(http://archives.postgresql.org/pgsql-sql/2006-06/msg00265.php)
On Fri, Feb 10, 2023 at 3:30 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Maybe for these cases where a WaitLatch is not desired, it'd be simpler
to do pg_usleep (5L * 1000 * 1000);
I somehow feel that we should be trying to get rid of cases where
WaitLatch is not desired.
That's probably overly simplistic - there might be cases where the
caller isn't just polling and has a really legitimate need to wait for
5 seconds of wall clock time. But even in that case, it seems like we
want to respond to barriers and interrupts during that time, in almost
all cases.
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.
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
I somehow feel that we should be trying to get rid of cases where
WaitLatch is not desired.
+1
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.
regards, tom lane
On Fri, Feb 10, 2023 at 10:18:34AM -0500, Tom Lane wrote:
Robert Haas <robertmhaas@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.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
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 <robertmhaas@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
Attachments:
pg_sleep.patchtext/x-diff; charset=us-asciiDownload
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 */
Nathan Bossart <nathandbossart@gmail.com> writes:
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:
BTW, we have an existing pg_sleep() function that deals with all
of this except re-setting the latch.
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).
A quick grep for pg_usleep with large intervals finds rather more
than you touched:
contrib/auth_delay/auth_delay.c: 46: pg_usleep(1000L * auth_delay_milliseconds);
src/backend/access/nbtree/nbtpage.c: 2979: pg_usleep(5000000L);
src/backend/access/transam/xlog.c: 5109: pg_usleep(60000000L);
src/backend/libpq/pqcomm.c: 717: pg_usleep(100000L); /* wait 0.1 sec */
src/backend/postmaster/bgwriter.c: 199: pg_usleep(1000000L);
src/backend/postmaster/checkpointer.c: 313: pg_usleep(1000000L);
src/backend/postmaster/checkpointer.c: 1009: pg_usleep(100000L); /* wait 0.1 sec, then retry */
src/backend/postmaster/postmaster.c: 4295: pg_usleep(PreAuthDelay * 1000000L);
src/backend/postmaster/walwriter.c: 192: pg_usleep(1000000L);
src/backend/postmaster/bgworker.c: 762: pg_usleep(PostAuthDelay * 1000000L);
src/backend/postmaster/pgarch.c: 456: pg_usleep(1000000L);
src/backend/postmaster/pgarch.c: 488: pg_usleep(1000000L); /* wait a bit before retrying */
src/backend/postmaster/autovacuum.c: 444: pg_usleep(PostAuthDelay * 1000000L);
src/backend/postmaster/autovacuum.c: 564: pg_usleep(1000000L);
src/backend/postmaster/autovacuum.c: 690: pg_usleep(1000000L); /* 1s */
src/backend/postmaster/autovacuum.c: 1711: pg_usleep(PostAuthDelay * 1000000L);
src/backend/storage/ipc/procarray.c: 3799: pg_usleep(100 * 1000L); /* 100ms */
src/backend/utils/init/postinit.c: 985: pg_usleep(PostAuthDelay * 1000000L);
src/backend/utils/init/postinit.c: 1192: pg_usleep(PostAuthDelay * 1000000L);
Did you have reasons for excluding the rest of these?
Taking a step back, I think it might be a mistake to try to share code
with the SQL-exposed function; at least, that is causing some API
decisions that feel odd. I have mixed emotions about both the use
of double as the sleep-time datatype, and about the choice of seconds
(rather than, say, msec) as the unit. Admittedly this is not all bad
--- for example, several of the calls I list above are delaying for
100ms, which we can easily accommodate in this scheme as "0.1", and
maybe it'd be a good idea to hit up the stuff waiting for 10ms too.
It still seems unusual for an internal support function though.
I haven't done the math on it, but are we limiting the precision
of the sleep (due to roundoff error) in any way that would matter?A bigger issue is that we almost certainly ought to pass through
a wait event code instead of allowing all these cases to be
WAIT_EVENT_PG_SLEEP.
I'd skip the unconditional latch reset you added to pg_sleep_sql.
I realize that's bug-compatible with what happens now, but I still
think it's expending extra code to do what might well be the
wrong thing.
We should update the header comment for pg_usleep to direct people
to this new function.
regards, tom lane
On Fri, Mar 10, 2023 at 12:28:28PM -0500, Tom Lane wrote:
A quick grep for pg_usleep with large intervals finds rather more
than you touched:[...]
Did you have reasons for excluding the rest of these?
I'm still looking into each of these, but my main concerns were 1) ensuring
latch support had been set up and 2) figuring out the right interrupt
function to use. Thus far, I don't think latch support is a problem
because InitializeLatchSupport() is called quite early. However, I'm not
as confident with the interrupt functions yet. Some of these multisecond
sleeps are done very early before the signal handlers are set up. Others
are done within the sigsetjmp() block. And at least one is in a code path
that both the startup process and single-user mode touch.
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 agree with the rest of your comments.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Mon, 13 Mar 2023 at 17:17, Nathan Bossart <nathandbossart@gmail.com> wrote:
On Fri, Mar 10, 2023 at 12:28:28PM -0500, Tom Lane wrote:
A quick grep for pg_usleep with large intervals finds rather more
than you touched:[...]
Did you have reasons for excluding the rest of these?
I'm still looking into each of these, but my main concerns were 1) ensuring
latch support had been set up and 2) figuring out the right interrupt
function to use. Thus far, I don't think latch support is a problem
because InitializeLatchSupport() is called quite early. However, I'm not
as confident with the interrupt functions yet. Some of these multisecond
sleeps are done very early before the signal handlers are set up. Others
are done within the sigsetjmp() block. And at least one is in a code path
that both the startup process and single-user mode touch.
Is this still a WIP? Is it targeting this release? There are only a
few days left before the feature freeze. I'm guessing it should just
move to the next CF for the next release?
--
Gregory Stark
As Commitfest Manager
On Mon, Apr 03, 2023 at 04:33:27PM -0400, Gregory Stark (as CFM) wrote:
Is this still a WIP? Is it targeting this release? There are only a
few days left before the feature freeze. I'm guessing it should just
move to the next CF for the next release?
I moved it to the next commitfest and marked the target version as v17.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On 4 Apr 2023, at 05:31, Nathan Bossart <nathandbossart@gmail.com> wrote:
On Mon, Apr 03, 2023 at 04:33:27PM -0400, Gregory Stark (as CFM) wrote:
Is this still a WIP? Is it targeting this release? There are only a
few days left before the feature freeze. I'm guessing it should just
move to the next CF for the next release?I moved it to the next commitfest and marked the target version as v17.
Have you had any chance to revisit this such that there is a new patch to
review, or should it be returned/moved for now?
--
Daniel Gustafsson
On Mon, Jul 10, 2023 at 10:12:36AM +0200, Daniel Gustafsson wrote:
Have you had any chance to revisit this such that there is a new patch to
review, or should it be returned/moved for now?
Not yet. I moved it to the next commitfest.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
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
Attachments:
v3-0001-WORK-IN-PROGRESS-pg_msleep.patchtext/x-diff; charset=us-asciiDownload
From 7ff61d6d1a4829e861a57bae621fb939f0133bc7 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@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
At Wed, 26 Jul 2023 16:41:25 -0700, Nathan Bossart <nathandbossart@gmail.com> wrote in
On Mon, Mar 13, 2023 at 02:16:31PM -0700, Nathan Bossart wrote:
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.
It is responsive to an immediate shutdown. I think that's fine, as
things might become overly complex if we aim for it to respond to fast
shutdown as well.
The pg_msleep() implemented in the patch accepts a wait event type,
and some event types other than WAIT_EVENT_PG_SLEEP are passed to it.
WAIT_EVENT_CHECKPOINTER_MAIN:
- At checkpointer.c:309, it is in a long-jump'ed block, where out of
the main loop.
- At checkpointer.c:1005: RequestCheckpoint is not executed on checkpointer.
WAIT_EVENT_ARCHIVER_MAIN:
- At pgarch.c:453,485: They are not at the main-loop level idle-waiting.
WAIT_EVENT_PRE/POST_AUTH_DELAY:
- These are really useless since they're not seen anywhere. Client
backends don't show up in pg_stat_activity until this sleep
finishes. (We could use ps-display instead, but...)
WAIT_EVENT_VACUUM_DELAY:
- This behaves the same as it did before the change. Actually, we
don't want to use WAIT_EVENT_PG_SLEEP for it.
So, we have at least one sensible use case for the parameter, which
seems to be sufficient reason.
I'm not sure if others will agree, but I'm leaning towards providing a
dedicated WaitEventSet for the new sleep function.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center