On Thu, Apr 15, 2021 at 11:48 AM Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> 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 <bharath.rupireddy@enterprisedb.com>
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 <bharath.rupireddy@enterprisedb.com>
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 <bharath.rupireddy@enterprisedb.com>
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
