Horton

Re: BitmapHeapScan streaming read user and prelim refactoring

Tomas Vondra <tomas@vondra.me>

View thread View on postgr.es
From: Tomas Vondra <tomas@vondra.me>
To: Melanie Plageman <melanieplageman@gmail.com>
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-10T18:11:04Z
Lists: pgsql-hackers 
On 2/10/25 19:02, Melanie Plageman wrote:
> On Sun, Feb 9, 2025 at 9:27 AM Tomas Vondra <tomas@vondra.me> wrote:
>>
>>
>> 2) ryzen
>> --------
>>
>> This "new" machine has multiple types of storage. The cached results (be
>> it in shared buffers or in page cache) are not very interesting. 0003
>> helps a bit (~15%), but other than that it's just random noise.
>>
>> The "uncached" results starting on page 23 are much more interesting. In
>> general 0001, 0002 and 0004 have little impact, it seems just random
>> noise. So in the rest I'll focus on 0003.
>>
>> For the single nvme device (device: data), it seems mostly fine. It's
>> green, even though there are a couple "localized regressions" for eic=0.
>> I haven't looked into those yet.
>>
>> For the nvme RAID (device: raid-nvme), it's looks almost exactly the
>> same, except that with parallel query (page 27) there's a clear area of
>> regression with eic=1 (look for "column" of red cells). That's a bit
>> unfortunate, because eic=1 is the default value.
> 
> It'll be hard to look into all of these, so I think I'll focus on
> trying to reproduce something with eic=1 that I can reproduce on my
> machine. So far, I can reproduce a regression with the following and
> the data file attached.
> 

Yes, that approach makes perfect sense. I don't think anyone can
investigate all the regressions, it's enough to investigate one specimen
for each of the main "patterns".

> # initdb and get set up with shared_buffers 1GB
> psql -c "create table bitmap_scan_test (a bigint, b bigint, c text)
> with (fillfactor = 25)"
> psql -c "copy bitmap_scan_test from '/tmp/bitmap_scan_test.data'"
> psql -c "create index on bitmap_scan_test (a)"
> psql -c "vacuum analyze"
> psql -c "checkpoint"
> 
> pg_ctl stop
> echo 3 | sudo tee /proc/sys/vm/drop_caches
> pg_ctl start
> psql -c "SET max_parallel_workers_per_gather = 4;" \
>       -c "SET effective_io_concurrency = 1;" \
>       -c "SET parallel_setup_cost = 0;" \
>       -c "SET parallel_tuple_cost = 0;" \
>       -c "SET enable_seqscan = off;" \
>       -c "SET enable_indexscan = off;" \
>       -c "SET work_mem = 65536;"
> 
> psql -c "EXPLAIN SELECT * FROM bitmap_scan_test WHERE (a BETWEEN -33
> AND 10015) OFFSET 1000000;"
> psql -c "SELECT * FROM bitmap_scan_test WHERE (a BETWEEN -33 AND
> 10015) OFFSET 1000000;"
> 
> It's not a huge regression and planner doesn't naturally pick parallel
> bitmap heap scan for this, but I don't have a SATA drive right now, so
> I focused on something I could reproduce.
> 
> One thing I noticed when I was playing around with the script is that
> depending on the values chosen by random(), there were differences in
> timing. From your script, it looks like the $from and $to won't be the
> same for master and the patch each time (they are set in the inner
> most nesting level, below where $build is set). Am I understanding
> correctly?
> 

Yes. The values for the WHERE conditions are generated randomly, and the
idea is that it evens out for multiple runs. Maybe not with only 3 runs
per query, but it should be good enough to show patterns (e.g. when the
runs with eic=1 show regression).

>> Anyway, the results look sensible. It might be good to investigate some
>> of the regressions, and I'll try doing that if I find the time. But I
>> don't think that's necessarily a blocker - every patch of this type will
>> have a hardware where the heuristics doesn't quite do the right thing by
>> default. Which is why we have GUCs to tune it if appropriate.
> 
> Yea, I definitely won't be able to look into all of the regressions.
> So, I guess we have to ask if we are willing to make the tradeoff.
> 

I'm at peace with that.

Certainly for the "localized" regressions, and cases when bitmapheapscan
would not be picked. The eic=1 case makes me a bit more nervous, because
it's default and affects NVMe storage. Would be good to know why is
that, or perhaps consider bumping up eic default. Not sure.

regards

-- 
Tomas Vondra

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.