Re: index prefetching

From: Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, Tomas Vondra <tomas(at)vondra(dot)me>, Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, Melanie Plageman <melanieplageman(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Georgios <gkokolatos(at)protonmail(dot)com>, Konstantin Knizhnik <knizhnik(at)garret(dot)ru>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Subject: Re: index prefetching
Date: 2025-08-13 12:08:53
Message-ID: CAN55FZ3juyw30eouDXe4uppqNOwAbHT-OCAsn3n__i7uiF3mbg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Tue, 12 Aug 2025 at 22:30, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
> On Tue, Aug 12, 2025 at 11:22 PM Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> wrote:
> > Unfortunately this doesn't work. We need to handle backwards I/O
> > combining in the StartReadBuffersImpl() function too as buffer indexes
> > won't have correct blocknums. Also, I think buffer forwarding of split
> > backwards I/O should be handled in a couple of places.
>
> Perhaps there could be a flag pending_read_backwards that can only
> become set with pending_read_nblocks goes from 1 to 2, and then a new
> flag stream->ios[x].backwards (in struct InProgressIO) that is set in
> read_stream_start_pending_read(). Then immediately after
> WaitReadBuffers(), we reverse the buffers it returned in place if that
> flag was set. Oh, I see, you were imagining a flag
> READ_BUFFERS_REVERSE that tells WaitReadBuffers() to do that
> internally. Hmm. Either way I don't think you need to consider the
> forwarded buffers because they will be reversed during a later call
> that includes them in *nblocks (output value), no?

I think the problem is that we are not sure whether we will run
WaitReadBuffers() or not. Let's say that we will process blocknums 25,
24, 23, 22, 21 and 20 so we combined these IOs. We set the
pending_read_backwards flag and sent this IO operation to the
StartReadBuffers(). Let's consider that 22 and 20 are cache hits and
the rest are cache misses. In that case, starting processing buffers
(inside StartReadBuffers()) from 20 will fail because we will try to
return that immediately since this is a first buffer and it is cache
hit.

I think something like this, we will pass the pending_read_backwards
to the StartReadBuffers() and it will start to process blocknums from
backwards because of the pending_read_backwards being true. So,
buffer[0] -> 25 ... buffer[2] -> 23 and we will stop there because 22
is a cache hit. Now, we will reverse these buffers so that buffer[0]
-> 23 ... buffer[2] -> 25, and then send this IO operation to the
WaitReadBuffers() and reverse these buffers again after
WaitReadBuffers(). The problem with that approach is that we need to
forward 22, 21 and 20 and pending_read_blocknum shouldn't change
because we are still at 20, processed buffers don't affect
pending_read_blocknum. And we need to preserve pending_read_backwards
until we process all forwarded buffers, otherwise we may try to
combine forward (pending_read_blocknum is 20 and the let's say next
blocknum from read_stream_get_block() is 21, we shouldn't do IO
combining in that case).

--
Regards,
Nazir Bilal Yavuz
Microsoft

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2025-08-13 12:10:46 cfbot mistakenly reports that a rebase is needed
Previous Message Aleksander Alekseev 2025-08-13 11:54:54 Re: [PATCH] Refactor SLRU to always use long file names