Re: pg_usleep for multisecond delays
At Wed, 26 Jul 2023 16:41:25 -0700, Nathan Bossart 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
Re: pg_usleep for multisecond delays
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
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(500L);
+ 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(6000L);
+ pg_msleep(6, WAIT_EVENT_PG_SLEEP);
#endif
/*
@@ -6710,7 +6710,7 @@ CreateCheckPoint(int flags)
{
do
{
- pg_usleep(1L); /* 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(1L); /* 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
Re: pg_usleep for multisecond delays
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
Re: pg_usleep for multisecond delays
> On 4 Apr 2023, at 05:31, Nathan Bossart 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
Re: pg_usleep for multisecond delays
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
Re: pg_usleep for multisecond delays
On Mon, 13 Mar 2023 at 17:17, Nathan Bossart 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
Re: pg_usleep for multisecond delays
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
Re: pg_usleep for multisecond delays
Nathan Bossart 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(500L); src/backend/access/transam/xlog.c: 5109:pg_usleep(6000L); src/backend/libpq/pqcomm.c: 717:pg_usleep(10L); /* wait 0.1 sec */ src/backend/postmaster/bgwriter.c: 199: pg_usleep(100L); src/backend/postmaster/checkpointer.c: 313: pg_usleep(100L); src/backend/postmaster/checkpointer.c: 1009:pg_usleep(10L); /* wait 0.1 sec, then retry */ src/backend/postmaster/postmaster.c: 4295: pg_usleep(PreAuthDelay * 100L); src/backend/postmaster/walwriter.c: 192:pg_usleep(100L); src/backend/postmaster/bgworker.c: 762: pg_usleep(PostAuthDelay * 100L); src/backend/postmaster/pgarch.c: 456: pg_usleep(100L); src/backend/postmaster/pgarch.c: 488: pg_usleep(100L);/* wait a bit before retrying */ src/backend/postmaster/autovacuum.c: 444: pg_usleep(PostAuthDelay * 100L); src/backend/postmaster/autovacuum.c: 564: pg_usleep(100L); src/backend/postmaster/autovacuum.c: 690: pg_usleep(100L);/* 1s */ src/backend/postmaster/autovacuum.c: 1711: pg_usleep(PostAuthDelay * 100L); src/backend/storage/ipc/procarray.c: 3799: pg_usleep(100 * 1000L); /* 100ms */ src/backend/utils/init/postinit.c: 985: pg_usleep(PostAuthDelay * 100L); src/backend/utils/init/postinit.c: 1192:pg_usleep(PostAuthDelay * 100L); 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
Re: pg_usleep for multisecond delays
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 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(500L);
+ 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(100L); /* 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 * 100L);
+ 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(100L);
+pg_sleep(1, HandlePgArchInterrupts);
continue;
}
@@ -482,7 +482,7 @@ pgarch_ArchiverCopyLoop(void)
xlog)));
return; /* give up archiving for now */
}
-pg_usleep(100L); /* 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
Re: pg_usleep for multisecond delays
On Fri, Feb 10, 2023 at 10:18:34AM -0500, Tom Lane wrote: > Robert Haas 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
Re: pg_usleep for multisecond delays
Robert Haas 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
Re: pg_usleep for multisecond delays
On Fri, Feb 10, 2023 at 3:30 AM Alvaro Herrera 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
Re: pg_usleep for multisecond delays
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(500L); > + 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)
Re: pg_usleep for multisecond delays
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 signature.asc Description: PGP signature
Re: pg_usleep for multisecond delays
At Thu, 9 Feb 2023 14:51:14 -0800, Nathan Bossart 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
Re: pg_usleep for multisecond delays
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
Re: pg_usleep for multisecond delays
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
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
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(500L);
+ 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(6000L);
+ 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 * 100L);
+ 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(100L);
+ 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(100L); /* 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 * 100L);
+ 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 * 100L);
+ 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(100L);
+ 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(100L);
+ 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(100L);
+pg_ssleep(1);
continue;
}
@@ -482,7 +482,7 @@ pgarch_ArchiverCopyLoop(void)
xlog)));
return; /* give up archiving for now */
}
-pg_usleep(100L); /* 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.)
*/
