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