Re: Checkpointer sync queue fills up / loops around pg_usleep() are bad - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Checkpointer sync queue fills up / loops around pg_usleep() are bad
Date
Msg-id 39FF3ED6-FE59-4723-887A-2D5968BBF1EF@anarazel.de
Whole thread Raw
In response to Re: Checkpointer sync queue fills up / loops around pg_usleep() are bad  (Thomas Munro <thomas.munro@gmail.com>)
Responses Re: Checkpointer sync queue fills up / loops around pg_usleep() are bad
List pgsql-hackers
Hi,

On February 27, 2022 4:19:21 PM PST, Thomas Munro <thomas.munro@gmail.com> wrote:
>With the attached, 027_stream_regress.pl drops from ~29.5s to ~19.6s
>on my FreeBSD workstation!

That's impressive - wouldn't have guessed it to make that much of a difference. I assume that running the tests on
freebsdfor an older pg with a similar s_b & max_wal_size  doesn't benefit as much? I wonder how much windows will
improve.


>It seems a little strange to introduce a new wait event that will very
>often appear into a stable branch, but ... it is actually telling the
>truth, so there is that.

In the back branches it needs to be at the end of the enum - I assume you intended that just to be for HEAD.

I wonder whether in HEAD we shouldn't make that sleep duration be computed from the calculation in IsOnSchedule...


>The sleep/poll loop in RegisterSyncRequest() may also have another
>problem.  The comment explains that it was a deliberate choice not to
>do CHECK_FOR_INTERRUPTS() here, which may be debatable, but I don't
>think there's an excuse to ignore postmaster death in a loop that
>presumably becomes infinite if the checkpointer exits.  I guess we
>could do:
>
>-               pg_usleep(10000L);
>+               WaitLatch(NULL, WL_EXIT_ON_PM_DEATH | WL_TIMEOUT, 10,
>WAIT_EVENT_SYNC_REQUEST);
>
>But... really, this should be waiting on a condition variable that the
>checkpointer broadcasts on when the queue goes from full to not full,
>no?  Perhaps for master only?

Looks worth improving, but yes, I'd not do it in the back branches.

I do think it's worth giving that sleep a proper wait event though, even in the back branches.

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.



pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Design of pg_stat_subscription_workers vs pgstats
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: Is it correct to update db state in control file as "shutting down" during end-of-recovery checkpoint?