From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Checkpointer sync queue fills up / loops around pg_usleep() are bad |
Date: | 2022-02-28 01:36:20 |
Message-ID: | 39FF3ED6-FE59-4723-887A-2D5968BBF1EF@anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On February 27, 2022 4:19:21 PM PST, Thomas Munro <thomas(dot)munro(at)gmail(dot)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 freebsd for 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.
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro Horiguchi | 2022-02-28 01:51:06 | Re: Is it correct to update db state in control file as "shutting down" during end-of-recovery checkpoint? |
Previous Message | Masahiko Sawada | 2022-02-28 01:18:33 | Re: Design of pg_stat_subscription_workers vs pgstats |