Message-ID: <ef92e929-fa35-4fea-acc2-259486a08bba@enterprisedb.com>
Date: Mon, 18 Mar 2024 16:55:12 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
Content-Language: en-US
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Dilip Kumar
 <dilipbalaut@gmail.com>, Robert Haas <robertmhaas@gmail.com>,
 Nazir Bilal Yavuz <byavuz81@gmail.com>, Andres Freund <andres@anarazel.de>,
 Pg Hackers <pgsql-hackers@postgresql.org>,
 Thomas Munro <thomas.munro@gmail.com>
References: <20240216173559.xiy5xcl5dqmsprns@liskov>
 <20240227015028.knohvy3spaqwk7lf@liskov>
 <20240227142230.nu3ytvcjwouvczlt@liskov>
 <5f3b9d59-0f43-419d-80ca-6d04c07cf61a@iki.fi>
 <CAFiTN-vALdOjGA4zyKVrw93e8oTpeFxJWUKaeCrQczVw=W4DAQ@mail.gmail.com>
 <CA+Tgmob_5W8=kxPKbjzjy_UWO1QXv0Z53jnvLABJwuU7S4zL+Q@mail.gmail.com>
 <CAFiTN-sdUxh3O_7fh=iEVpmvZkALjywOkYu0LnqJcfDZEQdhVg@mail.gmail.com>
 <cd50927a-8b19-4f0f-884d-609212c71c91@iki.fi>
 <CAFiTN-vx1H0tCckC2ueiNTj+ibLHMGF4HYZyv99neHZkAzbWbQ@mail.gmail.com>
 <abf05d48-cecc-4f5b-8645-f9119d88f557@iki.fi>
 <20240314181625.a7uigo5ujaogfd6x@liskov>
 <2ed1e06b-feae-4c61-9f58-3fcc358104c7@enterprisedb.com>
 <CAAKRu_Z81GE13PnZO1NrB2YD0aA_656hFh7vz87W+RFvZeHcYw@mail.gmail.com>
 <84bf5689-4524-4ba1-b25e-18018045e94d@enterprisedb.com>
 <CAAKRu_aNke9CKsB0Q740Bzk3Cnf2bBjakjEZipWJjHEewF9sNg@mail.gmail.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: 
 <CAAKRu_aNke9CKsB0Q740Bzk3Cnf2bBjakjEZipWJjHEewF9sNg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/ef92e929-fa35-4fea-acc2-259486a08bba%40enterprisedb.com>
Precedence: bulk


On 3/18/24 15:47, Melanie Plageman wrote:
> On Sun, Mar 17, 2024 at 3:21 PM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
>>
>> On 3/14/24 22:39, Melanie Plageman wrote:
>>> On Thu, Mar 14, 2024 at 5:26 PM Tomas Vondra
>>> <tomas.vondra@enterprisedb.com> wrote:
>>>>
>>>> On 3/14/24 19:16, Melanie Plageman wrote:
>>>>> On Thu, Mar 14, 2024 at 03:32:04PM +0200, Heikki Linnakangas wrote:
>>>>>> ...
>>>>>>
>>>>>> Ok, committed that for now. Thanks for looking!
>>>>>
>>>>> Attached v6 is rebased over your new commit. It also has the "fix" in
>>>>> 0010 which moves BitmapAdjustPrefetchIterator() back above
>>>>> table_scan_bitmap_next_block(). I've also updated the Streaming Read API
>>>>> commit (0013) to Thomas' v7 version from [1]. This has the update that
>>>>> we theorize should address some of the regressions in the bitmapheapscan
>>>>> streaming read user in 0014.
>>>>>
>>>>
>>>> Should I rerun the benchmarks with these new patches, to see if it
>>>> really helps with the regressions?
>>>
>>> That would be awesome!
>>>
>>
>> OK, here's a couple charts comparing the effect of v6 patches to master.
>> These are from 1M and 10M data sets, same as the runs presented earlier
>> in this thread (the 10M is still running, but should be good enough for
>> this kind of visual comparison).
> 
> Thanks for doing this!
> 
>> What is even more obvious is that 0014 behaves *VERY* differently. I'm
>> not sure if this is a good thing or a problem is debatable/unclear. I'm
>> sure we don't want to cause regressions, but perhaps those are due to
>> the prefetch issue discussed elsewhere in this thread (identified by
>> Andres and Melanie). There are also many cases that got much faster, but
>> the question is whether this is due to better efficiency or maybe the
>> new code being more aggressive in some way (not sure).
> 
> Are these with the default effective_io_concurrency (1)? If so, the
> "effective" prefetch distance in many cases will be higher with the
> streaming read code applied. With effective_io_concurrency 1,
> "max_ios" will always be 1, but the number of blocks prefetched may
> exceed this (up to MAX_BUFFERS_PER_TRANSFER) because the streaming
> read code is always trying to build bigger IOs. And, if prefetching,
> it will prefetch IOs not yet in shared buffers before reading them.
> 

No, it's a mix of runs with random combinations of these parameters:

dataset: uniform uniform_pages linear linear_fuzz cyclic cyclic_fuzz
workers: 0 4
work_mem: 128kB 4MB 64MB
eic: 0 1 8 16 32
selectivity: 0-100%

I can either share the data (~70MB of CSV) or generate charts for
results with some filter.

> It's hard to tell without going into a specific repro why this would
> cause some queries to be much slower. In the forced bitmapheapscan, it
> would make sense that more prefetching is worse -- which is why a
> bitmapheapscan plan wouldn't have been chosen. But in the optimal
> cases, it is unclear why it would be worse.
> 

Yes, not sure about the optimal cases. I'll wait for the 10M runs to
complete, and then we can look for some patterns.

> I don't think there is any way it could be the issue Andres
> identified, because there is only one iterator. Nothing to get out of
> sync. It could be that the fadvises are being issued too close to the
> reads and aren't effective enough at covering up read latency on
> slower, older hardware. But that doesn't explain why master would
> sometimes be faster.
> 

Ah, right, thanks for the clarification. I forgot the streaming read API
does not use the two-iterator approach.

> Probably the only thing we can do is get into a repro. It would, of
> course, be easiest to do this with a serial query. I can dig into the
> scripts you shared earlier and try to find a good repro. Because the
> regressions may have shifted with Thomas' new version, it would help
> if you shared a category (cyclic/uniform/etc, parallel or serial, eic
> value, work mem, etc) where you now see the most regressions.
> 

OK, I've restarted the tests for only 0012 and 0014 patches, and I'll
wait for these to complete - I don't want to be looking for patterns
until we have enough data to smooth this out.


regards

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