On Thu, Apr 15, 2021 at 11:48 AM Bharath Rupireddy <[email protected]> wrote: > > We definitely have replaced a lot of sleeps with latch.c primitives > > over the past few years, since we got WL_EXIT_ON_PM_DEATH and > > condition variables. There may be many more to improve... You > > mentioned autovacuum... yeah, Stephen fixed one of these with commit > > 4753ef37, but yeah it's not great to have those others in there... > > I have not looked at the commit 4753ef37 previously, but it > essentially addresses the problem with pg_usleep for vacuum delay. I'm > thinking we can also replace pg_usleep in below places based on the > fact that pg_usleep should be avoided in 1) short waits in a loop 2) > when wait time is dependent on user configurable parameters. And using > WaitLatch may require us to add wait event types to WaitEventTimeout > enum, but that's okay.
I'm attaching 3 patches that replace pg_usleep with WaitLatch: 0001 in lazy_truncate_heap, 0002 in do_pg_stop_backup and 0003 for Pre and Post Auth Delay. Regression tests pass with these patches. Please review them. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
From 4cd54df7e27efb5dd82751fd8c7d15c475ce4a40 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Mon, 19 Apr 2021 19:25:02 +0530 Subject: [PATCH v1] Use a WaitLatch for lock waiting in lazy_truncate_heap Instead of using pg_usleep() in lazy_truncate_heap(), use a WaitLatch. This has the advantage that we will realize if the postmaster has been killed since the last time we decided to sleep. --- doc/src/sgml/monitoring.sgml | 5 +++++ src/backend/access/heap/vacuumlazy.c | 6 +++++- src/backend/utils/activity/wait_event.c | 3 +++ src/include/utils/wait_event.h | 3 ++- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 5cf083bb77..65d51ce445 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -2249,6 +2249,11 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser <entry><literal>VacuumDelay</literal></entry> <entry>Waiting in a cost-based vacuum delay point.</entry> </row> + <row> + <entry><literal>VacuumTruncateLock</literal></entry> + <entry>Waiting to acquire exclusive lock on relation for truncation while + vacuuming.</entry> + </row> </tbody> </tgroup> </table> diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index e90fc18aa9..dcd598fca8 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -3188,7 +3188,11 @@ lazy_truncate_heap(LVRelState *vacrel) return; } - pg_usleep(VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL * 1000L); + (void) WaitLatch(MyLatch, + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, + VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL, + WAIT_EVENT_VACUUM_TRUNCATE_LOCK); + ResetLatch(MyLatch); } /* diff --git a/src/backend/utils/activity/wait_event.c b/src/backend/utils/activity/wait_event.c index 89b5b8b7b9..694cd48315 100644 --- a/src/backend/utils/activity/wait_event.c +++ b/src/backend/utils/activity/wait_event.c @@ -485,6 +485,9 @@ pgstat_get_wait_timeout(WaitEventTimeout w) case WAIT_EVENT_VACUUM_DELAY: event_name = "VacuumDelay"; break; + case WAIT_EVENT_VACUUM_TRUNCATE_LOCK: + event_name = "VacuumTruncateLock"; + break; /* no default case, so that compiler will warn */ } diff --git a/src/include/utils/wait_event.h b/src/include/utils/wait_event.h index 47accc5ffe..60b8ca76f7 100644 --- a/src/include/utils/wait_event.h +++ b/src/include/utils/wait_event.h @@ -140,7 +140,8 @@ typedef enum WAIT_EVENT_PG_SLEEP, WAIT_EVENT_RECOVERY_APPLY_DELAY, WAIT_EVENT_RECOVERY_RETRIEVE_RETRY_INTERVAL, - WAIT_EVENT_VACUUM_DELAY + WAIT_EVENT_VACUUM_DELAY, + WAIT_EVENT_VACUUM_TRUNCATE_LOCK } WaitEventTimeout; /* ---------- -- 2.25.1
From cf38b83a8c224bce7db730fe3a18ec897690f8ae Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Tue, 20 Apr 2021 06:18:15 +0530 Subject: [PATCH v1] Use a WaitLatch in do_pg_stop_backup Instead of using pg_usleep() in do_pg_stop_backup(), use a WaitLatch. This has the advantage that we will realize if the postmaster has been killed since the last time we decided to sleep. --- src/backend/access/transam/xlog.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index adfc6f67e2..a632a46a57 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -11605,6 +11605,8 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p) ((!backup_started_in_recovery && XLogArchivingActive()) || (backup_started_in_recovery && XLogArchivingAlways()))) { + Latch *latch; + XLByteToPrevSeg(stoppoint, _logSegNo, wal_segment_size); XLogFileName(lastxlogfilename, stoptli, _logSegNo, wal_segment_size); @@ -11615,6 +11617,11 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p) seconds_before_warning = 60; waits = 0; + if (backup_started_in_recovery) + latch = &XLogCtl->recoveryWakeupLatch; + else + latch = MyLatch; + while (XLogArchiveIsBusy(lastxlogfilename) || XLogArchiveIsBusy(histfilename)) { @@ -11627,9 +11634,11 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p) reported_waiting = true; } - pgstat_report_wait_start(WAIT_EVENT_BACKUP_WAIT_WAL_ARCHIVE); - pg_usleep(1000000L); - pgstat_report_wait_end(); + (void) WaitLatch(latch, + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, + 1000L, + WAIT_EVENT_BACKUP_WAIT_WAL_ARCHIVE); + ResetLatch(latch); if (++waits >= seconds_before_warning) { -- 2.25.1
From e89fcae1267d1b508a7891df48efc8f8a48cf74e Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Mon, 19 Apr 2021 19:27:53 +0530 Subject: [PATCH v1] Use a WaitLatch for Pre and Post Auth Delay Instead of using pg_usleep() for waiting PostAuthDelay and PreAuthDelay, use a WaitLatch. This has the advantage that we will realize if the postmaster has been killed since the last time we decided to sleep. --- doc/src/sgml/monitoring.sgml | 10 ++++++++++ src/backend/postmaster/autovacuum.c | 18 ++++++++++++++++-- src/backend/postmaster/bgworker.c | 8 +++++++- src/backend/postmaster/postmaster.c | 8 +++++++- src/backend/utils/activity/wait_event.c | 6 ++++++ src/backend/utils/init/postinit.c | 16 ++++++++++++++-- src/include/utils/wait_event.h | 2 ++ 7 files changed, 62 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 65d51ce445..22208e1b6d 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -2235,6 +2235,16 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser <entry>Waiting due to a call to <function>pg_sleep</function> or a sibling function.</entry> </row> + <row> + <entry><literal>PostAuthDelay</literal></entry> + <entry>Waiting on connection startup after authentication to allow attach + from a debugger.</entry> + </row> + <row> + <entry><literal>PreAuthDelay</literal></entry> + <entry>Waiting on connection startup before authentication to allow + attach from a debugger.</entry> + </row> <row> <entry><literal>RecoveryApplyDelay</literal></entry> <entry>Waiting to apply WAL during recovery because of a delay diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 83c584ddc8..f5669ef227 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -446,8 +446,15 @@ AutoVacLauncherMain(int argc, char *argv[]) ereport(DEBUG1, (errmsg_internal("autovacuum launcher started"))); + /* Apply PostAuthDelay */ if (PostAuthDelay) - pg_usleep(PostAuthDelay * 1000000L); + { + (void) WaitLatch(MyLatch, + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, + PostAuthDelay * 1000L, + WAIT_EVENT_POST_AUTH_DELAY); + ResetLatch(MyLatch); + } SetProcessingMode(InitProcessing); @@ -1706,8 +1713,15 @@ AutoVacWorkerMain(int argc, char *argv[]) ereport(DEBUG1, (errmsg_internal("autovacuum: processing database \"%s\"", dbname))); + /* Apply PostAuthDelay */ if (PostAuthDelay) - pg_usleep(PostAuthDelay * 1000000L); + { + (void) WaitLatch(MyLatch, + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, + PostAuthDelay * 1000L, + WAIT_EVENT_POST_AUTH_DELAY); + ResetLatch(MyLatch); + } /* And do an appropriate amount of work */ recentXid = ReadNextTransactionId(); diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index 11fc1b7863..3fa703c3f8 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -752,7 +752,13 @@ StartBackgroundWorker(void) /* Apply PostAuthDelay */ if (PostAuthDelay > 0) - pg_usleep(PostAuthDelay * 1000000L); + { + (void) WaitLatch(MyLatch, + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, + PostAuthDelay * 1000L, + WAIT_EVENT_POST_AUTH_DELAY); + ResetLatch(MyLatch); + } /* * Set up signal handlers. diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index b05db5a473..da019e6c5e 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -4313,7 +4313,13 @@ BackendInitialize(Port *port) * is not honored until after authentication.) */ if (PreAuthDelay > 0) - pg_usleep(PreAuthDelay * 1000000L); + { + (void) WaitLatch(MyLatch, + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, + PostAuthDelay * 1000L, + WAIT_EVENT_PRE_AUTH_DELAY); + ResetLatch(MyLatch); + } /* This flag will remain set until InitPostgres finishes authentication */ ClientAuthInProgress = true; /* limit visibility of log messages */ diff --git a/src/backend/utils/activity/wait_event.c b/src/backend/utils/activity/wait_event.c index 694cd48315..780f0cde73 100644 --- a/src/backend/utils/activity/wait_event.c +++ b/src/backend/utils/activity/wait_event.c @@ -476,6 +476,12 @@ pgstat_get_wait_timeout(WaitEventTimeout w) case WAIT_EVENT_PG_SLEEP: event_name = "PgSleep"; break; + case WAIT_EVENT_POST_AUTH_DELAY: + event_name = "PostAuthDelay"; + break; + case WAIT_EVENT_PRE_AUTH_DELAY: + event_name = "PreAuthDelay"; + break; case WAIT_EVENT_RECOVERY_APPLY_DELAY: event_name = "RecoveryApplyDelay"; break; diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 51d1bbef30..2a48fd4d5b 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -849,7 +849,13 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username, /* Apply PostAuthDelay as soon as we've read all options */ if (PostAuthDelay > 0) - pg_usleep(PostAuthDelay * 1000000L); + { + (void) WaitLatch(MyLatch, + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, + PostAuthDelay * 1000L, + WAIT_EVENT_POST_AUTH_DELAY); + ResetLatch(MyLatch); + } /* initialize client encoding */ InitializeClientEncoding(); @@ -1055,7 +1061,13 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username, /* Apply PostAuthDelay as soon as we've read all options */ if (PostAuthDelay > 0) - pg_usleep(PostAuthDelay * 1000000L); + { + (void) WaitLatch(MyLatch, + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, + PostAuthDelay * 1000L, + WAIT_EVENT_POST_AUTH_DELAY); + ResetLatch(MyLatch); + } /* * Initialize various default states that can't be set up until we've diff --git a/src/include/utils/wait_event.h b/src/include/utils/wait_event.h index 60b8ca76f7..024ed8cc07 100644 --- a/src/include/utils/wait_event.h +++ b/src/include/utils/wait_event.h @@ -138,6 +138,8 @@ typedef enum { WAIT_EVENT_BASE_BACKUP_THROTTLE = PG_WAIT_TIMEOUT, WAIT_EVENT_PG_SLEEP, + WAIT_EVENT_POST_AUTH_DELAY, + WAIT_EVENT_PRE_AUTH_DELAY, WAIT_EVENT_RECOVERY_APPLY_DELAY, WAIT_EVENT_RECOVERY_RETRIEVE_RETRY_INTERVAL, WAIT_EVENT_VACUUM_DELAY, -- 2.25.1
