Horton

Re: Streaming read-ready sequential scan code

Thomas Munro <thomas.munro@gmail.com>

View thread View on postgr.es
From: Thomas Munro <thomas.munro@gmail.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: David Rowley <dgrowleyml@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Pg Hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2024-04-05T23:28:19Z
Lists: pgsql-hackers

Attachments

On Sat, Apr 6, 2024 at 6:55 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> On Fri, Apr 5, 2024 at 12:15 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> > The interesting column is hot.  The 200ms->211ms regression is due to
> > the extra bookkeeping in the slow path.  The rejiggered fastpath code
> > fixes it for me, or maybe sometimes shows an extra 1ms.  Phew.  Can
> > you reproduce that?
>
> I am able to reproduce the fast path solving the issue using Heikki's
> example here [1] but in shared buffers (hot).
>
> master:                           25 ms
> stream read:                   29 ms
> stream read + fast path: 25 ms

Great, thanks.

> I haven't looked into or reviewed the memory prefetching part.
>
> While reviewing 0002, I realized that I don't quite see how
> read_stream_get_block() will be used in the fastpath -- which it
> claims in its comments.

Comments improved.

> Oh and why does READ_STREAM_DISABLE_FAST_PATH macro exist?

Just for testing purposes.  Behaviour should be identical to external
observers either way, it's just a hand-rolled specialisation for
certain parameters, and it's useful to be able to verify that and
measure the speedup.  I think it's OK to leave a bit of
developer/testing scaffolding in the tree if it's likely to be useful
again and especially if like this case it doesn't create any dead
code.  (Perhaps in later work we might find the right way to get the
compiler to do the specialisation?  It's so simple though.)

The occasional CI problem I mentioned turned out to be
read_stream_reset() remembering a little too much state across
rescans.  Fixed.

Thanks both for the feedback on the ring buffer tweaks.  Comments
updated.  Here is the full stack of patches I would like to commit
very soon, though I may leave the memory prefetching part out for a
bit longer to see if I can find any downside.

Commits

  1. Fix unfairness in all-cached parallel seq scan.

  2. Fix if/while thinko in read_stream.c edge case.

  3. Increase default vacuum_buffer_usage_limit to 2MB.

  4. Allow BufferAccessStrategy to limit pin count.

  5. Improve read_stream.c's fast path.

  6. Secondary refactor of heap scanning functions

  7. Preliminary refactor of heap scanning functions

  8. Add VACUUM/ANALYZE BUFFER_USAGE_LIMIT option