Horton

Re: BitmapHeapScan streaming read user and prelim refactoring

Melanie Plageman <melanieplageman@gmail.com>

View thread View on postgr.es
From: Melanie Plageman <melanieplageman@gmail.com>
To: Tomas Vondra <tomas@vondra.me>
Cc: Nazir Bilal Yavuz <byavuz81@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Thomas Munro <thomas.munro@gmail.com>, Andres Freund <andres@anarazel.de>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2025-02-20T16:36:28Z
Lists: pgsql-hackers 
On Sun, Feb 16, 2025 at 7:29 AM Tomas Vondra <tomas@vondra.me> wrote:
>
> On 2/16/25 02:15, Tomas Vondra wrote:
> >
> > ...
> >
> > OK, I've uploaded the results to the github repository as usual
> >
> >   https://github.com/tvondra/bitmapscan-tests/tree/main/20250214-184807
> >
> > and I've generated the same PDF reports, with the colored comparison.
> >
> > If you compare the pivot tables (I opened the "same" PDF from the two
> > runs and flip between them using alt-tab, which makes the interesting
> > regions easy to spot), the change is very clear.
> >
> > Disabling the sequential detection greatly reduces the scope of
> > regressions. That looks pretty great, IMO.
> >
> > It also seems to lose some speedups, especially with io_combine_limit=1
> > and eic=1. I'm not sure why, if that's expected, etc.

Yea, I don't know why these would change with vs without sequential detection.
Honestly, for serial bitmap heap scan, I would assume that eic 1,
io_combine_limit 1 would be the same as master (i.e. issuing at least
the one fadvise and no additional latency introduced by trying to
combine IOs). So both the speedup and slowdown are a bit surprising to
me.

> > There still remain areas of regression, but most of them are for cases
> > that'd use index scan (tiny fraction of rows scanned), or with
> > read-ahead=4096 (and not for the lower settings).
> >
> > The read-ahead dependence is actually somewhat interesting, because I
> > realized the RAID array has this set to 8192 by default, i.e. even
> > higher than 4096 where it regresses. I suppose mdadm does that, or
> > something, I don't know how the default is calculated. But I assume it
> > depends on the number of devices, so larger arrays might have even
> > higher read-ahead values.

Are the readahead regressions you are talking about the ones with
io_combine_limit > 1 and effective_io_concurrency 0 (at higher
readahead values)? With these, we expect that no fadvises and higher
readahead values means the OS will do good readahead for us. But
somehow the read combining seems to be interfering with this.

This is curious. It seems like it must have something to do with the
way the kernel is calculating readahead size. While the linux kernel
readahead readme [1] is not particularly easy to understand, there are
some parts that stick out to me:

* The size of the async tail is determined by subtracting the size that
 * was explicitly requested from the determined request size, unless
 * this would be less than zero - then zero is used.

I wonder if somehow the larger IO requests are foiling readahead.
Honestly, I don't know how far we can get trying to figure this out.
And explicitly reverse engineering this may backfire in other ways.

- Melanie

[1] https://github.com/torvalds/linux/blob/master/mm/readahead.c

Commits

  1. Fix bitmapheapscan incorrect recheck of NULL tuples

  2. Increase default maintenance_io_concurrency to 16

  3. Separate TBM[Shared|Private]Iterator and TBMIterateResult

  4. Improve read_stream.c advice for dense streams.

  5. Increase default effective_io_concurrency to 16

  6. Delay extraction of TIDBitmap per page offsets

  7. Add lossy indicator to TBMIterateResult

  8. Move BitmapTableScan per-scan setup into a helper

  9. Add and use BitmapHeapScanDescData struct

  10. Fix bitmap table scan crash on iterator release

  11. Bitmap Table Scans use unified TBMIterator

  12. Add common interface for TBMIterators

  13. Make table_scan_bitmap_next_block() async-friendly

  14. Move EXPLAIN counter increment to heapam_scan_bitmap_next_block

  15. Refactor tidstore.c iterator buffering.

  16. BitmapHeapScan: Remove incorrect assert and reset field

  17. Change BitmapAdjustPrefetchIterator to accept BlockNumber

  18. BitmapHeapScan: Use correct recheck flag for skip_fetch

  19. BitmapHeapScan: Push skip_fetch optimization into table AM

  20. BitmapHeapScan: postpone setting can_skip_fetch

  21. BitmapHeapScan: begin scan after bitmap creation

  22. Fix EXPLAIN Bitmap heap scan to count pages with no visible tuples

  23. Remove redundant snapshot copying from parallel leader to workers

  24. Remove some obsolete smgrcloseall() calls.

  25. Remove the "snapshot too old" feature.

  26. Compute XID horizon for page level index vacuum on primary.