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 <[email protected]> writes:
>>> I wonder if we should have a wrapper around WaitLatch() that documents
>>> that if the latch is set before the time expires, it will reset the
>>> latch and try again to wait for the remaining time, after checking for
>>> interrupts etc.
>>
>> Resetting the latch seems not very friendly for general-purpose use
>> ... although I guess we could set it again on the way out.
>>
>> BTW, we have an existing pg_sleep() function that deals with all
>> of this except re-setting the latch.
>
> That seems workable. I think it might also need to accept a function
> pointer for custom interrupt checking (e.g., archiver code should use
> HandlePgArchInterrupts()). I'll put something together if that sounds
> alright.
Here is a work-in-progress patch. I quickly scanned through my previous
patch and applied this new function everywhere it seemed safe to use (which
unfortunately is rather limited).
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 3feee28d19..d78ae26b78 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -2976,7 +2976,7 @@ _bt_pendingfsm_finalize(Relation rel, BTVacState *vstate)
* never be effective without some other backend concurrently consuming an
* XID.
*/
- pg_usleep(5000000L);
+ pg_sleep(5, NULL);
#endif
/*
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index ff6149a179..c1286e27c2 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -687,7 +687,7 @@ AutoVacLauncherMain(int argc, char *argv[])
* of a worker will continue to fail in the same way.
*/
AutoVacuumShmem->av_signal[AutoVacForkFailed] = false;
- pg_usleep(1000000L); /* 1s */
+ pg_sleep(1, HandleAutoVacLauncherInterrupts);
SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_WORKER);
continue;
}
@@ -1708,7 +1708,7 @@ AutoVacWorkerMain(int argc, char *argv[])
(errmsg_internal("autovacuum: processing database \"%s\"", dbname)));
if (PostAuthDelay)
- pg_usleep(PostAuthDelay * 1000000L);
+ pg_sleep(PostAuthDelay, NULL);
/* And do an appropriate amount of work */
recentXid = ReadNextTransactionId();
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e551af2905..20a0f05399 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -450,7 +450,7 @@ pgarch_ArchiverCopyLoop(void)
}
/* wait a bit before retrying */
- pg_usleep(1000000L);
+ pg_sleep(1, HandlePgArchInterrupts);
continue;
}
@@ -482,7 +482,7 @@ pgarch_ArchiverCopyLoop(void)
xlog)));
return; /* give up archiving for now */
}
- pg_usleep(1000000L); /* wait a bit before retrying */
+ pg_sleep(1, HandlePgArchInterrupts); /* wait a bit before retrying */
}
}
}
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 220ddb8c01..29a9daca8e 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -365,12 +365,18 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
/*
* pg_sleep - delay for N seconds
+ *
+ * If the caller provides a NULL 'check_interrupts' function,
+ * CHECK_FOR_INTERRUPTS() is used instead.
+ *
+ * This function is careful to set the latch before returning if it had to be
+ * reset.
*/
-Datum
-pg_sleep(PG_FUNCTION_ARGS)
+void
+pg_sleep(float8 secs, void (*check_interrupts) (void))
{
- float8 secs = PG_GETARG_FLOAT8(0);
float8 endtime;
+ bool latch_set = false;
/*
* We sleep using WaitLatch, to ensure that we'll wake up promptly if an
@@ -392,8 +398,12 @@ pg_sleep(PG_FUNCTION_ARGS)
{
float8 delay;
long delay_ms;
+ int rc;
- CHECK_FOR_INTERRUPTS();
+ if (check_interrupts == NULL)
+ CHECK_FOR_INTERRUPTS();
+ else
+ (*check_interrupts) ();
delay = endtime - GetNowFloat();
if (delay >= 600.0)
@@ -403,13 +413,30 @@ pg_sleep(PG_FUNCTION_ARGS)
else
break;
- (void) WaitLatch(MyLatch,
- WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
- delay_ms,
- WAIT_EVENT_PG_SLEEP);
- ResetLatch(MyLatch);
+ rc = WaitLatch(MyLatch,
+ WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ delay_ms,
+ WAIT_EVENT_PG_SLEEP);
+
+ if (rc & WL_LATCH_SET)
+ {
+ latch_set = true;
+ ResetLatch(MyLatch);
+ }
}
+ if (latch_set)
+ SetLatch(MyLatch);
+}
+
+/*
+ * pg_sleep_sql - SQL-callable version of pg_sleep()
+ */
+Datum
+pg_sleep_sql(PG_FUNCTION_ARGS)
+{
+ pg_sleep(PG_GETARG_FLOAT8(0), NULL);
+ ResetLatch(MyLatch); /* pg_sleep might've set latch before returning */
PG_RETURN_VOID();
}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index c0f2a8a77c..b0d6bd58c7 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6529,7 +6529,7 @@
prosrc => 'pg_ls_dir' },
{ oid => '2626', descr => 'sleep for the specified time in seconds',
proname => 'pg_sleep', provolatile => 'v', prorettype => 'void',
- proargtypes => 'float8', prosrc => 'pg_sleep' },
+ proargtypes => 'float8', prosrc => 'pg_sleep_sql' },
{ oid => '3935', descr => 'sleep for the specified interval',
proname => 'pg_sleep_for', prolang => 'sql', provolatile => 'v',
prorettype => 'void', proargtypes => 'interval',
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index c309e0233d..3c1fb84c53 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -495,4 +495,7 @@ extern void RestoreClientConnectionInfo(char *conninfo);
/* in executor/nodeHash.c */
extern size_t get_hash_memory_limit(void);
+/* in utils/adt/misc.c */
+extern void pg_sleep(float8 secs, void (*check_interrupts) (void));
+
#endif /* MISCADMIN_H */
