Re: Confine vacuum skip logic to lazy_scan_skip

Melanie Plageman <melanieplageman@gmail.com>

From: Melanie Plageman <melanieplageman@gmail.com>
To: Tomas Vondra <tomas@vondra.me>
Cc: Thomas Munro <thomas.munro@gmail.com>, Noah Misch <noah@leadboat.com>, vignesh C <vignesh21@gmail.com>, Andres Freund <andres@anarazel.de>, 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-05T22:26:49Z
Lists: pgsql-hackers

Attachments

On Sat, Jan 18, 2025 at 11:51 AM Tomas Vondra <tomas@vondra.me> wrote:
>
> Sure. I repeated the benchmark with v13, and it seems the behavior did
> change. I no longer see the "big" regression when most of the pages get
> updated (and need vacuuming).
>
> I can't be 100% sure this is due to changes in the patch, because I did
> some significant upgrades to the machine since that time - it has Ryzen
> 9900x instead of the ancient i5-2500k, new mobo/RAM/...  It's pretty
> much a new machine, I only kept the "old" SATA SSD RAID storage so that
> I can do some tests with non-NVMe.
>
> So there's a (small) chance the previous runs were hitting a bottleneck
> that does not exist on the new hardware.
>
> Anyway, just to make this information more complete, the machine now has
> this configuration:
>
> * Ryzen 9 9900x (12/24C), 64GB RAM
> * storage:
>   - data: Samsung SSD 990 PRO 4TB (NVMe)
>   - raid-nvme: RAID0 4x Samsung SSD 990 PRO 1TB (NVMe)
>   - raid-sata: RAID0 6x Intel DC3700 100GB (SATA)
>
> Attached is the script, raw results (CSV) and two PDFs summarizing the
> results as a pivot table for different test parameters. Compared to the
> earlier run I tweaked the script to also vary io_combine_limit (ioc), as
> I wanted to see how it interacts with effective_io_concurrency (eic).
>
> Looking at the new results, I don't see any regressions, except for two
> cases - data (single NVMe) and raid-nvme (4x NVMe). There's a small area
> of regression for eic=32 and perc=0.0005, but only with WAL-logging.
>
> I'm not sure this is worth worrying about too much. It's a heuristics
> and for every heuristics there's some combination parameters where it
> doesn't quite do the optimal thing. The area where the patch brings
> massive improvements (or does not regress) are much more significant.
>
> I personally am happy with this behavior, seems to be performing fine.

Yes, looking at these results, I also feel good about it. I've updated
the commit metadata in attached v14, but I could use a round of review
before pushing it.

- 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.