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
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)
{
signature.asc
Description: PGP signature
