Horton

Re: Confine vacuum skip logic to lazy_scan_skip

Melanie Plageman <melanieplageman@gmail.com>

View thread View on postgr.es
From: Melanie Plageman <melanieplageman@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, Ranier Vilela <ranier.vf@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Tomas Vondra <tomas@vondra.me>, Noah Misch <noah@leadboat.com>, vignesh C <vignesh21@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>, Heikki Linnakangas <hlinnaka@iki.fi>, Nazir Bilal Yavuz <byavuz81@gmail.com>, Robert Haas <robertmhaas@gmail.com>, "Andrey M. Borodin" <x4mmm@yandex-team.ru>
Date: 2025-02-27T22:57:50Z
Lists: pgsql-hackers 
On Thu, Feb 27, 2025 at 1:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wonder if it'd be a good idea to add something like
>
>                 Assert(stream->distance == 1);
>                 Assert(stream->pending_read_nblocks == 0);
>                 Assert(stream->per_buffer_data_size == 0);
> +               Assert(per_buffer_data == NULL);
>
> in read_stream_next_buffer.  I doubt that this will shut Coverity
> up, but it would help to catch caller coding errors, i.e. passing
> a per_buffer_data pointer when there's no per-buffer data.

I think this is a good stopgap. I was discussing adding this assert
off-list with Thomas and he wanted to detail his more ambitious plans
for type safety improvements in the read stream API. Less on the order
of a redesign and more like a separate read_stream_next_buffer()s for
when there is per buffer data and when there isn't. And a by-value and
by-reference version for the one where there is data.

I'll plan to add this assert tomorrow if that discussion doesn't materialize.

- Melanie

Commits

  1. Fix explicit valgrind interaction in read_stream.c.

  2. Reduce scope of heap vacuum per_buffer_data

  3. Use streaming read I/O in VACUUM's third phase

  4. Use streaming read I/O in VACUUM's first phase

  5. Convert heap_vac_scan_next_block() boolean parameters to flags

  6. Refactor tidstore.c iterator buffering.

  7. Increase default vacuum_buffer_usage_limit to 2MB.

  8. Remove unneeded vacuum_delay_point from heap_vac_scan_get_next_block

  9. Confine vacuum skip logic to lazy_scan_skip()

  10. Set all_visible_according_to_vm correctly with DISABLE_PAGE_SKIPPING

  11. Tighten up VACUUM's approach to setting VM bits.