Can a child process detect postmaster death when in pg_usleep?
Hi,
In my dev system(Ubuntu) when the postmaster is killed with SIGKILL,
SIGPWR is being sent to its child processes (backends/any other bg
process). If a child process is waiting with pg_usleep, it looks like
it is not detecting the postmaster's death and it doesn't exit
immediately but stays forever until it gets killed explicitly. For
this experiment, I did 2 things to simulate the scenario i.e. a
backend waiting in pg_usleep and killing the postmaster. 1) I wrote a
wait function that uses pg_usleep and called it in a backend. This
backend doesn't exit on postmaster death. 2) I set PostAuthDelay to
100 seconds and started the postmaster. Then, the "auotvacuum
launcher" process still stays around (as it has pg_usleep in its main
function), even after postmaster death.
Questions:
1) Is it really harmful to use pg_usleep in a postmaster child process
as it doesn't let the child process detect postmaster death?
2) Can pg_usleep() always detect signals? I see the caution in the
pg_usleep function definition in pgsleep.c, saying the signal handling
is platform dependent. We have code blocks like below in the code. Do
we actually process interrupts before going to sleep with pg_usleep()?
while/for loop
{
......
......
CHECK_FOR_INTERRUPTS();
pg_usleep();
}
and
if (PostAuthDelay)
pg_usleep();
3) Is it intentional to use pg_usleep in some places in the code? If
yes, what are they? At least, I see one place where it's intentional
in the wait_pid function which is used while running the regression
tests.
4) Are there any places where we need to replace pg_usleep with
WaitLatch/equivalent of pg_sleep to detect the postmaster death
properly?
Correct me if I'm missing something or if my observation/understanding
of the pg_usleep() is wrong.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Hi Bharath,
On Thu, Apr 15, 2021 at 2:06 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
1) Is it really harmful to use pg_usleep in a postmaster child process
as it doesn't let the child process detect postmaster death?
Yeah, that's a bad idea. Any long-term waiting (including short waits
in a loop) should ideally be done with the latch infrastructure.
One interesting and unusual case is recovery: it can run for a very
long time without reaching a waiting primitive of any kind (other than
LWLock, which doesn't count), because it can be busy applying records
for hours at a time. In that case, we take special measures and
explicitly check if the postmaster is dead in the redo loop. In
theory, you could do the same in any loop containing pg_usleep() (we
used to have several loops doing that, especially around replication
code), but it'd be better to use the existing wait-event-multiplexing
technology we have, and keep improving that.
Some people have argued that long running queries should *also* react
faster when the PM exits, a bit like recovery ... which leads to the
next point...
2) Can pg_usleep() always detect signals? I see the caution in the
pg_usleep function definition in pgsleep.c, saying the signal handling
is platform dependent. We have code blocks like below in the code. Do
we actually process interrupts before going to sleep with pg_usleep()?
while/for loop
{
......
......
CHECK_FOR_INTERRUPTS();
pg_usleep();
}
and
if (PostAuthDelay)
pg_usleep();
CHECK_FOR_INTERRUPTS() has nothing to do with postmaster death
detection, currently, so that'd be for dealing with interrupts, not
for that. Also, there would be a race: a signal on its own isn't
enough on systems where we have them and where select() is guaranteed
to wake up, because the signal might arrive between CFI() and
pg_usleep(100 years). latch.c knows how to void such problems.
There may be an argument that CFI() *should* be a potential
postmaster-death-exit point, instead of having WaitLatch() (or its
caller) handle it directly, but it's complicated. At the time the
postmaster pipe system was invented we didn't have a signals for this
so it wasn't even a candidate for treatment as an "interrupt". On
systems that have postmaster death signals today (Linux + FreeBSD, but
I suspect we can extend this to every Unix we support, see CF #3066,
and a solution for Windows has been mentioned too), clearly the signal
handler could set a new interrupt flag PostmasterLost +
InterruptPending, and then CHECK_FOR_INTERRUPTS() could see it and
exit. The argument against this is that exiting isn't always the
right thing! In a couple of places, we do something special, such as
printing a special error message (examples: sync rep and the main FEBE
client read). Look for WL_POSTMASTER_DEATH (as opposed to
WL_EXIT_ON_PM_DEATH). So I guess you'd need to reverse those
decisions and standardise on "exit immediately, no message", or
invented a way to suppress that behaviour in code regions.
3) Is it intentional to use pg_usleep in some places in the code? If
yes, what are they? At least, I see one place where it's intentional
in the wait_pid function which is used while running the regression
tests.
There are plenty of places that do a short sleep for various reasons,
more like a deliberate stall or backoff or auth thing, and it's
probably OK if they're shortish and not really a condition polling
loop with an obvious latch/CV-based replacement. Note also that
LWLock waits are similar.
4) Are there any places where we need to replace pg_usleep with
WaitLatch/equivalent of pg_sleep to detect the postmaster death
properly?
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...
On Thu, Apr 15, 2021 at 5:28 AM Thomas Munro <thomas.munro@gmail.com> wrote:
Thanks a lot for the detailed explanation.
On Thu, Apr 15, 2021 at 2:06 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:1) Is it really harmful to use pg_usleep in a postmaster child process
as it doesn't let the child process detect postmaster death?Yeah, that's a bad idea. Any long-term waiting (including short waits
in a loop) should ideally be done with the latch infrastructure.
Agree. Along with short waits in a loop, I think we also should
replace pg_usleep with WaitLatch that has a user configurable
parameter like below:
pg_usleep(VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL * 1000L);
pg_usleep(PostAuthDelay * 1000000L);
pg_usleep(CommitDelay);
4) Are there any places where we need to replace pg_usleep with
WaitLatch/equivalent of pg_sleep to detect the postmaster death
properly?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.
1) pg_usleep(VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL * 1000L); in lazy_truncate_heap
2) pg_usleep(CommitDelay); in XLogFlush
3) pg_usleep(10000L); in CreateCheckPoint
4) pg_usleep(1000000L); in do_pg_stop_backup
5) pg_usleep(1000L); in read_local_xlog_page
6) pg_usleep(PostAuthDelay * 1000000L); in AutoVacLauncherMain,
AutoVacWorkerMain, StartBackgroundWorker, InitPostgres
7) pg_usleep(100000L); in RequestCheckpoint
8) pg_usleep(1000000L); in pgarch_ArchiverCopyLoop
9) pg_usleep(PGSTAT_RETRY_DELAY * 1000L); in backend_read_statsfile
10) pg_usleep(PreAuthDelay * 1000000L); in BackendInitialize
11) pg_usleep(10000L); in WalSndWaitStopping
12) pg_usleep(standbyWait_us); in WaitExceedsMaxStandbyDelay
13) pg_usleep(10000L); in RegisterSyncRequest
I'm sure we won't be changing in all of the above places. It will be
good to review and correct the above list.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
On Thu, Apr 15, 2021 at 11:48 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@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
Attachments:
v1-0001-Use-a-WaitLatch-for-lock-waiting-in-lazy_truncate.patchapplication/x-patch; name=v1-0001-Use-a-WaitLatch-for-lock-waiting-in-lazy_truncate.patchDownload
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
v1-0002-Use-a-WaitLatch-in-do_pg_stop_backup.patchapplication/x-patch; name=v1-0002-Use-a-WaitLatch-in-do_pg_stop_backup.patchDownload
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
v1-0003-Use-a-WaitLatch-for-Pre-and-Post-Auth-Delay.patchapplication/x-patch; name=v1-0003-Use-a-WaitLatch-for-Pre-and-Post-Auth-Delay.patchDownload
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
On Tue, Apr 20, 2021 at 7:36 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
On Thu, Apr 15, 2021 at 11:48 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@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.
I made a CF entry [1]https://commitfest.postgresql.org/33/3085/ so that it may get a chance for review.
[1]: https://commitfest.postgresql.org/33/3085/
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed
This patch looks fine. Tested on MacOS Catalina; master 09ae3299
The new status of this patch is: Ready for Committer
On Tue, Apr 20, 2021 at 07:36:39AM +0530, Bharath Rupireddy wrote:
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.
+ if (backup_started_in_recovery)
+ latch = &XLogCtl->recoveryWakeupLatch;
+ else
+ latch = MyLatch;
recoveryWakeupLatch is used by the startup process, but it has nothing
to do with do_pg_stop_backup(). Why are you doing that?I can get behind the change for the truncation lock when finishing a
VACUUM as that helps with monitoring. Now, I am not sure I get the
point of changing anything for {post,pre}_auth_delay that are
developer options. Please note that at this stage we don't know the
backend activity in pg_stat_activity, so the use of wait events is not
really interesting. On top of that, not reacting on signals can be
interesting to keep as a behavior for developers?
--
Michael
On Thu, Jun 24, 2021 at 12:04 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Apr 20, 2021 at 07:36:39AM +0530, Bharath Rupireddy wrote:
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.+ if (backup_started_in_recovery) + latch = &XLogCtl->recoveryWakeupLatch; + else + latch = MyLatch; recoveryWakeupLatch is used by the startup process, but it has nothing to do with do_pg_stop_backup(). Why are you doing that?
The recoveryWakeupLatch and procLatch/MyLatch are being used for WAL
replay and recovery conflict, respectively. Actually, I was earlier
using procLatch/MyLatch, but came across the commit 00f690a23 which
says that the two latches are reserved for specific purposes. I'm not
quite sure which one to use when do_pg_stop_backup is called by the
startup process. Any thoughts?
I can get behind the change for the truncation lock when finishing a
VACUUM as that helps with monitoring.
Thanks. Please let me know if there are any comments on
v1-0001-Use-a-WaitLatch-for-lock-waiting-in-lazy_truncate.patch.
Now, I am not sure I get the
point of changing anything for {post,pre}_auth_delay that are
developer options. Please note that at this stage we don't know the
backend activity in pg_stat_activity, so the use of wait events is not
really interesting.
Hm. I was earlier thinking from the perspective that the processes
should be able to detect the postmaster death if the
{post,pre}_auth_delay are set to higher values. Now, I agree that the
auth delays are happening at the initial stages of the processes and
if the developers(not common users) set the higher values for the
GUCs, let them deal with the problem of the processes not detecting
the postmaster death.
On top of that, not reacting on signals can be
interesting to keep as a behavior for developers?
Yeah, it can be useful at times as it enables debugging even when the
postmaster dies.
With Regards,
Bharath Rupireddy.
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
On Thu, Jun 24, 2021 at 12:04 PM Michael Paquier <michael@paquier.xyz> wrote:
On top of that, not reacting on signals can be
interesting to keep as a behavior for developers?
Yeah, it can be useful at times as it enables debugging even when the
postmaster dies.
Dunno ... I cannot recall ever having had that as a debugging requirement
in a couple of decades worth of PG bug-chasing. If the postmaster is
dying, you generally want to deal with that before bothering with child
processes. Moreover, child processes that don't go awy when the
postmaster does are a very nasty problem, because they could screw up
subsequent debugging work.
regards, tom lane
On Mon, Jun 28, 2021 at 11:01:57AM -0400, Tom Lane wrote:
Dunno ... I cannot recall ever having had that as a debugging requirement
in a couple of decades worth of PG bug-chasing. If the postmaster is
dying, you generally want to deal with that before bothering with child
processes. Moreover, child processes that don't go awy when the
postmaster does are a very nasty problem, because they could screw up
subsequent debugging work.
At the same time, nobody has really complained about this being an
issue for developer options. I would tend to wait for more opinions
before doing anything with the auth_delay GUCs.
--
Michael
On Mon, Jun 28, 2021 at 08:21:06PM +0530, Bharath Rupireddy wrote:
The recoveryWakeupLatch and procLatch/MyLatch are being used for WAL
replay and recovery conflict, respectively. Actually, I was earlier
using procLatch/MyLatch, but came across the commit 00f690a23 which
says that the two latches are reserved for specific purposes. I'm not
quite sure which one to use when do_pg_stop_backup is called by the
startup process. Any thoughts?
Could you explain why you think dp_pg_stop_backup() can be called by
the startup process? AFAIK, this code path applies to two categories
of sessions:
- backend sessions, with the SQL functions calling this routine.
- WAL senders, aka anything that connects with replication=1 able to
use the BASE_BACKUP with the replication protocol.
Thanks. Please let me know if there are any comments on
v1-0001-Use-a-WaitLatch-for-lock-waiting-in-lazy_truncate.patch.
Applied this one as that's clearly a win. The event name has been
renamed to VacuumTruncate.
--
Michael
On Fri, Jul 2, 2021 at 9:53 AM Michael Paquier <michael@paquier.xyz> wrote:
On Mon, Jun 28, 2021 at 08:21:06PM +0530, Bharath Rupireddy wrote:
The recoveryWakeupLatch and procLatch/MyLatch are being used for WAL
replay and recovery conflict, respectively. Actually, I was earlier
using procLatch/MyLatch, but came across the commit 00f690a23 which
says that the two latches are reserved for specific purposes. I'm not
quite sure which one to use when do_pg_stop_backup is called by the
startup process. Any thoughts?Could you explain why you think dp_pg_stop_backup() can be called by
the startup process? AFAIK, this code path applies to two categories
of sessions:
- backend sessions, with the SQL functions calling this routine.
- WAL senders, aka anything that connects with replication=1 able to
use the BASE_BACKUP with the replication protocol.
My bad. I was talking about the cases when do_pg_stop_backup is called
while the server is in recovery mode i.e. backup_started_in_recovery =
RecoveryInProgress(); evaluates to true. I'm not sure in these cases
whether we should replace pg_usleep with WaitLatch. If yes, whether we
should use procLatch/MyLatch or recoveryWakeupLatch as they are
currently serving different purposes.
Regards,
Bharath Rupireddy.
On Fri, Jul 02, 2021 at 12:03:07PM +0530, Bharath Rupireddy wrote:
My bad. I was talking about the cases when do_pg_stop_backup is called
while the server is in recovery mode i.e. backup_started_in_recovery =
RecoveryInProgress(); evaluates to true. I'm not sure in these cases
whether we should replace pg_usleep with WaitLatch. If yes, whether we
should use procLatch/MyLatch or recoveryWakeupLatch as they are
currently serving different purposes.
It seems to me that you should re-read the description of
recoveryWakeupLatch at the top of xlog.c and check for which purpose
it exists, which is, in this case, to wake up the startup process to
accelerate WAL replay. So do_pg_stop_backup() has no business with
it.
Switching pg_stop_backup() to use a latch rather than pg_usleep() has
benefits:
- It simplifies the wait event handling.
- The process waiting for the last WAL segment to be archived will be
more responsive on signals like SIGHUP and on postmaster death.
These don't sound bad to me to apply here, so 0002 could be simplified
as attached.
--
Michael
Attachments:
stop-backup-latch-v2.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7890e13d7a..c7c928f50b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -11638,9 +11638,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(MyLatch,
+ WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ 1000L,
+ WAIT_EVENT_BACKUP_WAIT_WAL_ARCHIVE);
+ ResetLatch(MyLatch);
if (++waits >= seconds_before_warning)
{
At Fri, 2 Jul 2021 10:27:21 +0900, Michael Paquier <michael@paquier.xyz> wrote in
On Mon, Jun 28, 2021 at 11:01:57AM -0400, Tom Lane wrote:
Dunno ... I cannot recall ever having had that as a debugging requirement
in a couple of decades worth of PG bug-chasing. If the postmaster is
dying, you generally want to deal with that before bothering with child
processes. Moreover, child processes that don't go awy when the
postmaster does are a very nasty problem, because they could screw up
subsequent debugging work.At the same time, nobody has really complained about this being an
issue for developer options. I would tend to wait for more opinions
before doing anything with the auth_delay GUCs.
I'm not sure the current behavior is especially useful for debugging,
however, I don't think it is especially useful that children
immediately respond to postmaster's death while the debug-delays,
because anyway children don't respond while debugging (until the
control (or code-pointer) reaches to the point of checking
postmaster's death), and the delays must be very short even if someone
abuses it on production systems. On the other hand, there could be a
discussion as a convention that any user-definable sleep requires to
respond to signals, maybe as Thomas mentioned.
So, I don't object either way we will go. But if we don't change the
behavior we instead would need a comment that explains the reason for
the pg_usleep.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Greetings,
* Michael Paquier (michael@paquier.xyz) wrote:
On Fri, Jul 02, 2021 at 12:03:07PM +0530, Bharath Rupireddy wrote:
My bad. I was talking about the cases when do_pg_stop_backup is called
while the server is in recovery mode i.e. backup_started_in_recovery =
RecoveryInProgress(); evaluates to true. I'm not sure in these cases
whether we should replace pg_usleep with WaitLatch. If yes, whether we
should use procLatch/MyLatch or recoveryWakeupLatch as they are
currently serving different purposes.It seems to me that you should re-read the description of
recoveryWakeupLatch at the top of xlog.c and check for which purpose
it exists, which is, in this case, to wake up the startup process to
accelerate WAL replay. So do_pg_stop_backup() has no business with
it.Switching pg_stop_backup() to use a latch rather than pg_usleep() has
benefits:
- It simplifies the wait event handling.
- The process waiting for the last WAL segment to be archived will be
more responsive on signals like SIGHUP and on postmaster death.
Yes, agreed.
These don't sound bad to me to apply here, so 0002 could be simplified
as attached.
Took a quick look and the patch looks good to me.
In general, I agree with Tom's up-thread comment about children hanging
around after postmaster death making things more difficult for debugging
and just in general, so I'm in favor of trying to eliminate as many
cases where that's happening as we reasonably can without impacting
performance by checking too often.
Thanks,
Stephen
On Mon, Jul 5, 2021 at 7:33 AM Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Jul 02, 2021 at 12:03:07PM +0530, Bharath Rupireddy wrote:
My bad. I was talking about the cases when do_pg_stop_backup is called
while the server is in recovery mode i.e. backup_started_in_recovery =
RecoveryInProgress(); evaluates to true. I'm not sure in these cases
whether we should replace pg_usleep with WaitLatch. If yes, whether we
should use procLatch/MyLatch or recoveryWakeupLatch as they are
currently serving different purposes.It seems to me that you should re-read the description of
recoveryWakeupLatch at the top of xlog.c and check for which purpose
it exists, which is, in this case, to wake up the startup process to
accelerate WAL replay. So do_pg_stop_backup() has no business with
it.
Hm. The shared recoveryWakeupLatch is being owned by the startup
process to wait and other backends/processes are using it to wake up
the startup process.
Switching pg_stop_backup() to use a latch rather than pg_usleep() has
benefits:
- It simplifies the wait event handling.
- The process waiting for the last WAL segment to be archived will be
more responsive on signals like SIGHUP and on postmaster death.These don't sound bad to me to apply here, so 0002 could be simplified
as attached.
The attached stop-backup-latch-v2.patch looks good to me.
Regards,
Bharath Rupireddy.
On Mon, Jul 5, 2021 at 9:25 PM Stephen Frost <sfrost@snowman.net> wrote:
In general, I agree with Tom's up-thread comment about children hanging
around after postmaster death making things more difficult for debugging
and just in general, so I'm in favor of trying to eliminate as many
cases where that's happening as we reasonably can without impacting
performance by checking too often.
I agree. I'm attaching the patch that replaces pg_usleep with
WaitLatch for {pre, post}_auth_delay. I'm also attaching Michael's
latest patch stop-backup-latch-v2.patch, just for the sake of cfbot.
Regards,
Bharath Rupireddy.
Attachments:
stop-backup-latch-v2.patchapplication/octet-stream; name=stop-backup-latch-v2.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7890e13d7a..c7c928f50b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -11638,9 +11638,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(MyLatch,
+ WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ 1000L,
+ WAIT_EVENT_BACKUP_WAIT_WAL_ARCHIVE);
+ ResetLatch(MyLatch);
if (++waits >= seconds_before_warning)
{
v1-0003-Use-a-WaitLatch-for-Pre-and-Post-Auth-Delay.patchapplication/octet-stream; name=v1-0003-Use-a-WaitLatch-for-Pre-and-Post-Auth-Delay.patchDownload
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
On Mon, Jul 05, 2021 at 09:42:29PM +0530, Bharath Rupireddy wrote:
I agree. I'm attaching the patch that replaces pg_usleep with
WaitLatch for {pre, post}_auth_delay. I'm also attaching Michael's
latest patch stop-backup-latch-v2.patch, just for the sake of cfbot.
I don't object to the argument that switching to a latch for this code
path could be good for responsiveness, but switching it is less
attractive than the others as wait events are not available in
pg_stat_activity at authentication startup. That's the case of normal
backends and WAL senders, not the case of autovacuum workers using
post_auth_delay if I read the code correctly.
Anyway, it is worth noting that the patch as proposed breaks
post_auth_delay. MyLatch is set when reaching WaitLatch() for
post_auth_delay after loading the options, so the use of WL_LATCH_SET
is not right. I think that this comes from SwitchToSharedLatch() in
InitProcess(). And it does not seem quite right to me to just blindly
reset the latch before doing the wait in this code path. Perhaps we
could just use (WL_TIMEOUT | WL_EXIT_ON_PM_DEATH) to do the job.
The one for pg_stop_backup() has been applied, no objections to that.
--
Michael
On Tue, Jul 6, 2021 at 6:15 AM Michael Paquier <michael@paquier.xyz> wrote:
On Mon, Jul 05, 2021 at 09:42:29PM +0530, Bharath Rupireddy wrote:
I agree. I'm attaching the patch that replaces pg_usleep with
WaitLatch for {pre, post}_auth_delay. I'm also attaching Michael's
latest patch stop-backup-latch-v2.patch, just for the sake of cfbot.I don't object to the argument that switching to a latch for this code
path could be good for responsiveness, but switching it is less
attractive than the others as wait events are not available in
pg_stat_activity at authentication startup. That's the case of normal
backends and WAL senders, not the case of autovacuum workers using
post_auth_delay if I read the code correctly.
We may not see anything in the pg_stat_activity for {post,
pre}_auth_delay, but the processes can detect the postmaster death
with WaitLatch. I think we should focus on that.
Anyway, it is worth noting that the patch as proposed breaks
post_auth_delay. MyLatch is set when reaching WaitLatch() for
post_auth_delay after loading the options, so the use of WL_LATCH_SET
is not right. I think that this comes from SwitchToSharedLatch() in
InitProcess(). And it does not seem quite right to me to just blindly
reset the latch before doing the wait in this code path. Perhaps we
could just use (WL_TIMEOUT | WL_EXIT_ON_PM_DEATH) to do the job.
I'm sorry to say that I didn't get what was said above. We reset the
latch after we come out of WaitLatch but not before going to wait. And
the reason to have WL_LATCH_SET, is to exit the wait loop if MyLatch
is set for that process because of other SetLatch events. Am I missing
something here?
Regards,
Bharath Rupireddy.
On Tue, Jul 06, 2021 at 12:42:21PM +0530, Bharath Rupireddy wrote:
I'm sorry to say that I didn't get what was said above. We reset the
latch after we come out of WaitLatch but not before going to wait. And
the reason to have WL_LATCH_SET, is to exit the wait loop if MyLatch
is set for that process because of other SetLatch events. Am I missing
something here?
Did you test the patch with post_auth_delay and a backend connection,
making sure that the delay gets correctly applied? I did, and that
was not working here.
--
Michael
On Tue, Jul 6, 2021 at 1:38 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Jul 06, 2021 at 12:42:21PM +0530, Bharath Rupireddy wrote:
I'm sorry to say that I didn't get what was said above. We reset the
latch after we come out of WaitLatch but not before going to wait. And
the reason to have WL_LATCH_SET, is to exit the wait loop if MyLatch
is set for that process because of other SetLatch events. Am I missing
something here?Did you test the patch with post_auth_delay and a backend connection,
making sure that the delay gets correctly applied? I did, and that
was not working here.
Thanks. You are right. The issue is due to the MyLatch being set by
SwitchToSharedLatch before WaitLatch. If we use (WL_TIMEOUT |
WL_EXIT_ON_PM_DEATH), then the backends will honour the
post_auth_delay as well as detect the postmaster death. Since we are
not using WL_LATCH_SET, I removed ResetLatch. Also, added some
comments around why we are not using WL_LATCH_SET.
For PreAuthDelay, there's no problem to use WL_LATCH_SET as MyLatch
still points to the local latch(which is not set) in
BackendInitialize().
PSA v2 patch.
Regards,
Bharath Rupireddy.
Attachments:
v2-0001-Use-a-WaitLatch-for-Pre-and-Post-Auth-Delay.patchapplication/octet-stream; name=v2-0001-Use-a-WaitLatch-for-Pre-and-Post-Auth-Delay.patchDownload
From a2bc7372b0a9072f2ac9cb927273084d98f623f3 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Tue, 6 Jul 2021 10:17:22 +0000
Subject: [PATCH v2] 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.
For PostAuthDelay, don't use WL_LATCH_SET. Because MyLatch, on
which WaitLatch going to wait, could have been set in
SwitchToSharedLatch() by InitProcess(). For PreAuthDelay, there's
no problem to use WL_LATCH_SET as MyLatch still points to local
latch in BackendInitialize().
---
doc/src/sgml/monitoring.sgml | 10 ++++++++++
src/backend/postmaster/autovacuum.c | 24 ++++++++++++++++++++++--
src/backend/postmaster/bgworker.c | 11 ++++++++++-
src/backend/postmaster/postmaster.c | 8 +++++++-
src/backend/utils/activity/wait_event.c | 6 ++++++
src/backend/utils/init/postinit.c | 22 ++++++++++++++++++++--
src/include/utils/wait_event.h | 2 ++
7 files changed, 77 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 643e1ad49f..3f04704832 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -2228,6 +2228,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 912ef9cb54..336dde07b2 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -446,8 +446,18 @@ AutoVacLauncherMain(int argc, char *argv[])
ereport(DEBUG1,
(errmsg_internal("autovacuum launcher started")));
+ /* Apply PostAuthDelay */
if (PostAuthDelay)
- pg_usleep(PostAuthDelay * 1000000L);
+ {
+ /*
+ * PostAuthDelay will not get applied, if WL_LATCH_SET is used. This
+ * is because the latch could have been set initially.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PostAuthDelay * 1000L,
+ WAIT_EVENT_POST_AUTH_DELAY);
+ }
SetProcessingMode(InitProcessing);
@@ -1706,8 +1716,18 @@ AutoVacWorkerMain(int argc, char *argv[])
ereport(DEBUG1,
(errmsg_internal("autovacuum: processing database \"%s\"", dbname)));
+ /* Apply PostAuthDelay */
if (PostAuthDelay)
- pg_usleep(PostAuthDelay * 1000000L);
+ {
+ /*
+ * PostAuthDelay will not get applied, if WL_LATCH_SET is used.
+ * This is because the latch could have been set initially.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PostAuthDelay * 1000L,
+ WAIT_EVENT_POST_AUTH_DELAY);
+ }
/* And do an appropriate amount of work */
recentXid = ReadNextTransactionId();
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index c40410d73e..9c97a5d1f6 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -763,7 +763,16 @@ StartBackgroundWorker(void)
/* Apply PostAuthDelay */
if (PostAuthDelay > 0)
- pg_usleep(PostAuthDelay * 1000000L);
+ {
+ /*
+ * PostAuthDelay will not get applied, if WL_LATCH_SET is used. This
+ * is because the latch could have been set initially.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PostAuthDelay * 1000L,
+ WAIT_EVENT_POST_AUTH_DELAY);
+ }
/*
* Set up signal handlers.
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 5a050898fe..0a808b510e 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4332,7 +4332,13 @@ BackendInitialize(Port *port)
* is not honored until after authentication.)
*/
if (PreAuthDelay > 0)
- pg_usleep(PreAuthDelay * 1000000L);
+ {
+ ret = WaitLatch(MyLatch,
+ WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PreAuthDelay * 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 ef7e6bfb77..9b2835bef8 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..c5491f635f 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -849,7 +849,16 @@ 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);
+ {
+ /*
+ * PostAuthDelay will not get applied, if WL_LATCH_SET is used.
+ * This is because the latch could have been set initially.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PostAuthDelay * 1000L,
+ WAIT_EVENT_POST_AUTH_DELAY);
+ }
/* initialize client encoding */
InitializeClientEncoding();
@@ -1055,7 +1064,16 @@ 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);
+ {
+ /*
+ * PostAuthDelay will not get applied, if WL_LATCH_SET is used. This
+ * is because the latch could have been set initially.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PostAuthDelay * 1000L,
+ WAIT_EVENT_POST_AUTH_DELAY);
+ }
/*
* 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 6007827b44..1154f537b3 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
On Tue, Jul 06, 2021 at 03:54:07PM +0530, Bharath Rupireddy wrote:
Thanks. You are right. The issue is due to the MyLatch being set by
SwitchToSharedLatch before WaitLatch. If we use (WL_TIMEOUT |
WL_EXIT_ON_PM_DEATH), then the backends will honour the
post_auth_delay as well as detect the postmaster death. Since we are
not using WL_LATCH_SET, I removed ResetLatch. Also, added some
comments around why we are not using WL_LATCH_SET.For PreAuthDelay, there's no problem to use WL_LATCH_SET as MyLatch
still points to the local latch(which is not set) in
BackendInitialize().
FWIW, I think that it could be a good idea to use the same set of
flags for all the pre/post_auth_delay paths for consistency. That's
useful when grepping for one. Please note that I don't plan to look
more at this patch set for this CF as I am not really excited by the
updates involving developer options, and I suspect more issues like
the one I found upthread so this needs a close lookup.
If somebody else wishes to look at it, please feel free, of course.
--
Michael
On Tue, Jul 6, 2021 at 4:33 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Jul 06, 2021 at 03:54:07PM +0530, Bharath Rupireddy wrote:
Thanks. You are right. The issue is due to the MyLatch being set by
SwitchToSharedLatch before WaitLatch. If we use (WL_TIMEOUT |
WL_EXIT_ON_PM_DEATH), then the backends will honour the
post_auth_delay as well as detect the postmaster death. Since we are
not using WL_LATCH_SET, I removed ResetLatch. Also, added some
comments around why we are not using WL_LATCH_SET.For PreAuthDelay, there's no problem to use WL_LATCH_SET as MyLatch
still points to the local latch(which is not set) in
BackendInitialize().FWIW, I think that it could be a good idea to use the same set of
flags for all the pre/post_auth_delay paths for consistency. That's
useful when grepping for one. Please note that I don't plan to look
more at this patch set for this CF as I am not really excited by the
updates involving developer options, and I suspect more issues like
the one I found upthread so this needs a close lookup.If somebody else wishes to look at it, please feel free, of course.
Thanks. Anyways, I removed WL_LATCH_SET for PreAuthDelay as well. PSA v4 patch.
Regards,
Bharath Rupireddy.
Attachments:
v3-0001-Use-a-WaitLatch-for-Pre-and-Post-Auth-Delay.patchapplication/octet-stream; name=v3-0001-Use-a-WaitLatch-for-Pre-and-Post-Auth-Delay.patchDownload
From 27fc7000943bfddc4a4df3ca0fce6b02fb6a94b7 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Tue, 6 Jul 2021 11:34:08 +0000
Subject: [PATCH v3] 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.
For PostAuthDelay, don't use WL_LATCH_SET. Because MyLatch, on
which WaitLatch going to wait, could have been set in
SwitchToSharedLatch() by InitProcess(). For PreAuthDelay, although
there's no problem to use WL_LATCH_SET as MyLatch still points to local
latch in BackendInitialize(), let's not use it as the MyLatch may
point to shared latch later.
---
doc/src/sgml/monitoring.sgml | 10 ++++++++++
src/backend/postmaster/autovacuum.c | 24 ++++++++++++++++++++++--
src/backend/postmaster/bgworker.c | 11 ++++++++++-
src/backend/postmaster/postmaster.c | 11 ++++++++++-
src/backend/utils/activity/wait_event.c | 6 ++++++
src/backend/utils/init/postinit.c | 22 ++++++++++++++++++++--
src/include/utils/wait_event.h | 2 ++
7 files changed, 80 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 643e1ad49f..3f04704832 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -2228,6 +2228,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 912ef9cb54..336dde07b2 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -446,8 +446,18 @@ AutoVacLauncherMain(int argc, char *argv[])
ereport(DEBUG1,
(errmsg_internal("autovacuum launcher started")));
+ /* Apply PostAuthDelay */
if (PostAuthDelay)
- pg_usleep(PostAuthDelay * 1000000L);
+ {
+ /*
+ * PostAuthDelay will not get applied, if WL_LATCH_SET is used. This
+ * is because the latch could have been set initially.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PostAuthDelay * 1000L,
+ WAIT_EVENT_POST_AUTH_DELAY);
+ }
SetProcessingMode(InitProcessing);
@@ -1706,8 +1716,18 @@ AutoVacWorkerMain(int argc, char *argv[])
ereport(DEBUG1,
(errmsg_internal("autovacuum: processing database \"%s\"", dbname)));
+ /* Apply PostAuthDelay */
if (PostAuthDelay)
- pg_usleep(PostAuthDelay * 1000000L);
+ {
+ /*
+ * PostAuthDelay will not get applied, if WL_LATCH_SET is used.
+ * This is because the latch could have been set initially.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PostAuthDelay * 1000L,
+ WAIT_EVENT_POST_AUTH_DELAY);
+ }
/* And do an appropriate amount of work */
recentXid = ReadNextTransactionId();
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index c40410d73e..9c97a5d1f6 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -763,7 +763,16 @@ StartBackgroundWorker(void)
/* Apply PostAuthDelay */
if (PostAuthDelay > 0)
- pg_usleep(PostAuthDelay * 1000000L);
+ {
+ /*
+ * PostAuthDelay will not get applied, if WL_LATCH_SET is used. This
+ * is because the latch could have been set initially.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PostAuthDelay * 1000L,
+ WAIT_EVENT_POST_AUTH_DELAY);
+ }
/*
* Set up signal handlers.
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 5a050898fe..f880eaf682 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4332,7 +4332,16 @@ BackendInitialize(Port *port)
* is not honored until after authentication.)
*/
if (PreAuthDelay > 0)
- pg_usleep(PreAuthDelay * 1000000L);
+ {
+ /*
+ * Do not use WL_LATCH_SET during backend initialization because the
+ * MyLatch may point to shared latch later.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PreAuthDelay * 1000L,
+ WAIT_EVENT_PRE_AUTH_DELAY);
+ }
/* 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 ef7e6bfb77..9b2835bef8 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..c5491f635f 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -849,7 +849,16 @@ 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);
+ {
+ /*
+ * PostAuthDelay will not get applied, if WL_LATCH_SET is used.
+ * This is because the latch could have been set initially.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PostAuthDelay * 1000L,
+ WAIT_EVENT_POST_AUTH_DELAY);
+ }
/* initialize client encoding */
InitializeClientEncoding();
@@ -1055,7 +1064,16 @@ 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);
+ {
+ /*
+ * PostAuthDelay will not get applied, if WL_LATCH_SET is used. This
+ * is because the latch could have been set initially.
+ */
+ (void) WaitLatch(MyLatch,
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PostAuthDelay * 1000L,
+ WAIT_EVENT_POST_AUTH_DELAY);
+ }
/*
* 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 6007827b44..1154f537b3 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
On Tue, Jul 06, 2021 at 05:07:04PM +0530, Bharath Rupireddy wrote:
Thanks. Anyways, I removed WL_LATCH_SET for PreAuthDelay as
well. PSA v4 patch.
For the moment, please note that I have marked the patch as committed
in the CF app. It may be better to start a new thread with the
remaining bits for a separate evaluation.
--
Michael