Horton

Re: BitmapHeapScan streaming read user and prelim refactoring

Tomas Vondra <tomas.vondra@enterprisedb.com>

View thread View on postgr.es
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Pg Hackers <pgsql-hackers@postgresql.org>, Thomas Munro <thomas.munro@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: 2024-03-02T15:05:07Z
Lists: pgsql-hackers

Attachments


On 3/1/24 18:08, Tomas Vondra wrote:
> 
> On 3/1/24 17:51, Melanie Plageman wrote:
>> On Fri, Mar 1, 2024 at 9:05 AM Tomas Vondra
>> <tomas.vondra@enterprisedb.com> wrote:
>>>
>>> On 3/1/24 02:18, Melanie Plageman wrote:
>>>> On Thu, Feb 29, 2024 at 6:44 PM Tomas Vondra
>>>> <tomas.vondra@enterprisedb.com> wrote:
>>>>>
>>>>> On 2/29/24 23:44, Tomas Vondra wrote:
>>>>> 1) On master there's clear difference between eic=0 and eic=1 cases, but
>>>>> on the patched build there's literally no difference - for example the
>>>>> "uniform" distribution is clearly not great for prefetching, but eic=0
>>>>> regresses to eic=1 poor behavior).
>>>>
>>>> Yes, so eic=0 and eic=1 are identical with the streaming read API.
>>>> That is, eic 0 does not disable prefetching. Thomas is going to update
>>>> the streaming read API to avoid issuing an fadvise for the last block
>>>> in a range before issuing a read -- which would mean no prefetching
>>>> with eic 0 and eic 1. Not doing prefetching with eic 1 actually seems
>>>> like the right behavior -- which would be different than what master
>>>> is doing, right?
>>>
>>> I don't think we should stop doing prefetching for eic=1, or at least
>>> not based just on these charts. I suspect these "uniform" charts are not
>>> a great example for the prefetching, because it's about distribution of
>>> individual rows, and even a small fraction of rows may match most of the
>>> pages. It's great for finding strange behaviors / corner cases, but
>>> probably not a sufficient reason to change the default.
>>
>> Yes, I would like to see results from a data set where selectivity is
>> more correlated to pages/heap fetches. But, I'm not sure I see how
>> that is related to prefetching when eic = 1.
>>
> 
> OK, I'll make that happen.
> 

Here's a PDF with charts for a dataset where the row selectivity is more
correlated to selectivity of pages. I'm attaching the updated script,
with the SQL generating the data set. But the short story is all rows on
a single page have the same random value, so the selectivity of rows and
pages should be the same.

The first page has results for the original "uniform", the second page
is the new "uniform-pages" data set. There are 4 charts, for
master/patched and 0/4 parallel workers. Overall the behavior is the
same, but for the "uniform-pages" it's much more gradual (with respect
to row selectivity). I think that's expected.


As for how this is related to eic=1 - I think my point was that these
are "adversary" data sets, most likely to show regressions. This applies
especially to the "uniform" data set, because as the row selectivity
grows, it's more and more likely it's right after to the current one,
and so a read-ahead would likely do the trick.

Also, this is forcing a bitmap scan plan - it's possible many of these
cases would use some other scan type, making the regression somewhat
irrelevant. Not entirely, because we make planning mistakes and for
robustness reasons it's good to keep the regression small.

But that's just how I think about it now. I don't think I have some
grand theory that'd dictate we have to do prefetching for eic=1.

regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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.