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.
