Re: Improve read_local_xlog_page_guts by replacing polling with latch-based waiting

On Thu, Oct 02, 2025 at 11:06:14PM +0800, Xuneng Zhou wrote:
> v5-0002 separates the waitlsn_cmp() comparator function into two distinct
> functions (waitlsn_replay_cmp and waitlsn_flush_cmp) for the replay
> and flush heaps, respectively.

The primary goal that you want to achieve here is a replacement of the
wait/sleep logic of read_local_xlog_page_guts() with a condition
variable, and design a new facility to make the callback more
responsive on polls.  That's a fine idea in itself.  However I would
suggest to implement something that does not depend entirely on WAIT
FOR, which is how your patch is presented.  Instead of having your
patch depend on an entirely different feature, it seems to me that you
should try to extract from this other feature the basics that you are
looking for, and make them shared between the WAIT FOR patch and what
you are trying to achieve here.  You should not need something as
complex as what the other feature needs for a page read callback in
the backend.

At the end, I suspect that you will reuse a slight portion of it (or
perhaps nothing at all, actually, but I did not look at the full scope
of it).  You should try to present your patch so as it is in a
reviewable state, so as others would be able to read it and understand
it.  WAIT FOR is much more complex than what you want to do here
because it covers larger areas of the code base and needs to worry
about more cases.  So, you should implement things so as the basic
pieces you want to build on top of are simpler, not more complicated.
Simpler means easier to review and easier to catch problems, designed
in a way that depends on how you want to fix your problem, not
designed in a way that depends on how a completely different feature
deals with its own problems.
--
Michael