Hi,
On Sun, Aug 10, 2025 at 10:01 AM Thomas Munro <thomas.munro@gmail.com> wrote:
>
> On Sun, Aug 10, 2025 at 12:22 PM Michael Paquier <michael@paquier.xyz> wrote:
> > With all these comments and b421223172a2 already applied, are you sure
> > that it is a good idea to play with the v18 branch more than
> > necessary? We are in a baked beta2 state, and it looks like all these
> > could qualify as HEAD-only improvements.
>
> Yeah, b421223172a2 closes the known bug and thus the critical path item for 18.
>
> I am happy to propose this patch as an improvement for master, and I
> am not aware of remaining bugs in this area in v18. I just wanted to
> publish the patch with analysis ASAP once the existing
> read_stream.c/bufmgr.c interaction began to seem egregiously
> suboptimal to me and I saw what to do about, having addressed the bug
> with a low risk patch as a first priority. That keeps the options
> open, even if not very wide given the date: on the off-chance that
> others think the status quo isn't good enough and perhaps the
> fragility of the existing API is actually riskier than the bigger
> change, at least there's a patch on the table to discuss. I have no
> plans to take any action without such a consensus.
>
> > Perhaps the open item can be closed then?
>
> Done.
I read the high level explanation in the prior email; it's a pretty
dense reading. I wouldn't claim to have a very good understanding of
it, but the one line clean-up seems to fix the bug well. This overall
idea of the follow-up patch looks sound; however it feels more like
performance improvement enabled by enhanced design. I also did a rough
read on the patch itself and found two minor typos.
"the output value can be used directly (as input as input) to the following
call along with the buffers it counts."
"On entry, *nblocks is the (desire) number of block to read."
Best,
Xuneng
