From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
Cc: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Andres Freund <andres(at)anarazel(dot)de>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> |
Subject: | Re: BitmapHeapScan streaming read user and prelim refactoring |
Date: | 2024-04-07 14:38:46 |
Message-ID: | CAAKRu_Yaoi17Hz-uP4zSwQZ-06LeKxeq1fAp32E_HrfOF5iWKg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Apr 7, 2024 at 10:10 AM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>
> On 4/7/24 15:11, Melanie Plageman wrote:
>
> > Also, the iterators in the TableScanDescData might be something I
> > could live with in the source code for a couple months before we make
> > the rest of the changes in July+. But, adding them does push the
> > TableScanDescData->rs_parallel member into the second cacheline, which
> > will be true in versions of Postgres people are using for years. I
> > didn't perf test, but seems bad.
> >
>
> I haven't though about how it affects cachelines, TBH. I'd expect it to
> have minimal impact, because while it makes this struct larger it should
> make some other struct (used in essentially the same places) smaller. So
> I'd guess this to be a zero sum game, but perhaps I'm wrong.
Yea, to be honest, I didn't do extensive analysis. I just ran `pahole
-C TableScanDescData` with the patch and on master and further
convinced myself the whole thing was a bad idea.
> For me the main question was "Is this the right place for this, even if
> it's only temporary?"
Yep.
> > So, yes, unfortunately, I think we should pick up on the BHS saga in a
> > few months. Or, actually, we should start focusing on that parallel
> > BHS + 0 readahead bug and whether or not we are going to fix it.
> >
>
> Yes, the July CF is a good time to focus on this, early in the cycle.
I've pushed the entry to the next CF. Even though some of the patches
were committed, I think it makes more sense to leave the CF item open
until at least the prefetch table AM violation is removed. Then we
could make a new CF entry for the streaming read user.
When I get a chance, I'll post a full set of the outstanding patches
to this thread -- including the streaming read-related refactoring and
user.
Oh, and, side note, in my previous email [1] about the
empty_tuples_pending assert, I should mention that you need
set enable_seqscan = off
set enable_indexscan = off
to force bitmpaheapscan and get the plan for my fake repro.
- Melanie
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2024-04-07 14:41:59 | Re: BitmapHeapScan streaming read user and prelim refactoring |
Previous Message | Melanie Plageman | 2024-04-07 14:24:38 | Re: BitmapHeapScan streaming read user and prelim refactoring |