Re: BitmapHeapScan streaming read user and prelim refactoring

Melanie Plageman <melanieplageman@gmail.com>

From: Melanie Plageman <melanieplageman@gmail.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Thomas Munro <thomas.munro@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Andres Freund <andres@anarazel.de>, Pg Hackers <pgsql-hackers@postgresql.org>, Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: 2024-06-17T21:22:12Z
Lists: pgsql-hackers

Attachments

On Fri, Jun 14, 2024 at 7:56 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> Attached v21 is the rest of the patches to make bitmap heap scan use
> the read stream API.
---snip---
> The one relic of iterator ownership is that for parallel bitmap heap
> scan, a single process must scan the index, construct the bitmap, and
> call tbm_prepare_shared_iterate() to set up the iterator for all the
> processes. I didn't see a way to push this down (because to build the
> bitmap we have to scan the index and call ExecProcNode()). I wonder if
> this creates an odd split of responsibilities. I could use some other
> synchronization mechanism to communicate which process built the
> bitmap in the generic bitmap table scan code and thus should set up
> the iterator in the heap implementation, but that sounds like a pretty
> bad idea. Also, I'm not sure how a non-block-based table AM would use
> TBMIterators (or even TIDBitmap) anyway.

I tinkered around with this some more and actually came up with a
solution to my primary concern with the code structure. Attached is
v22. It still needs the performance regression investigation mentioned
in my previous email, but I feel more confident in the layering of the
iterator ownership I ended up with.

Because the patch numbers have changed, below is a summary of the
contents with the new patch numbers:

Patches 0001-0003 implement the async-friendly behavior needed both to
push down the VM lookups for prefetching and eventually to use the
read stream API.

Patches 0004-0006 add and make use of a common interface for the
shared and private bitmap iterators per Heikki's
suggestion in [1].

Patches 0008 - 0012 make new scan descriptors for bitmap table scans
and the heap AM implementation.

0013 and 0014 push all of the prefetch code down into heap AM code as
suggested by Heikki in [1].

0016 removes scan_bitmap_next_block() per Heikki's suggestion in [1].

Patches 0017 and 0018 make some changes to the TIDBitmap API to
support having multiple TBMIterateResults at the same time instead of
reusing the same one when iterating.

0019 uses the read stream API and removes all the bespoke prefetching
code from bitmap heap scan.

- Melanie

[1] https://www.postgresql.org/message-id/5a172d1e-d69c-409a-b1fa-6521214c81c2%40iki.fi

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.