Hi,
On 2025-04-02 18:58:11 +0200, Matthias van de Meent wrote:
> And here it is, v6 of the patchset, rebased up to master @ 764d501d.
Thanks!
Does anybody have an opinion about how non-invasive to be in the
back-branches? The minimal version is something like this diff:
diff --git i/src/backend
On Wed, 2 Apr 2025 at 17:37, Andres Freund wrote:
>
> Hi,
>
> Matthias, any chance you can provide a rebased version for master?
Sure, I'll try to have it in your inbox later today CEST.
> Either way I'm planning to push this fairly soon.
OK, thanks!
Kind regards,
Matthias van de Meent
Neon (
Hi,
Matthias, any chance you can provide a rebased version for master? If not,
I'll do the rebase. Either way I'm planning to push this fairly soon.
Greetings,
Andres Freund
Andres Freund writes:
> Does anybody have an opinion about how non-invasive to be in the
> back-branches? The minimal version is something like this diff:
Minimal is good -- less chance of breaking anything.
> - Should we commit the test showing that the naive implementation of
> index-only-bi
Hi,
On 2025-04-02 14:17:01 -0400, Tom Lane wrote:
> Andres Freund writes:
> > Does anybody have an opinion about how non-invasive to be in the
> > back-branches? The minimal version is something like this diff:
>
> Minimal is good -- less chance of breaking anything.
>
> > - Should we commit th
On Wed, 2 Apr 2025 at 19:48, Andres Freund wrote:
>
> Hi,
>
> On 2025-04-02 18:58:11 +0200, Matthias van de Meent wrote:
> > And here it is, v6 of the patchset, rebased up to master @ 764d501d.
>
> Thanks!
>
> Does anybody have an opinion about how non-invasive to be in the
> back-branches? The mi
On Wed, 2 Apr 2025 at 18:20, Matthias van de Meent
wrote:
>
> On Wed, 2 Apr 2025 at 17:37, Andres Freund wrote:
> >
> > Hi,
> >
> > Matthias, any chance you can provide a rebased version for master?
>
> Sure, I'll try to have it in your inbox later today CEST.
And here it is, v6 of the patchset,
Hi,
On 2025-03-17 16:17:03 +0100, Matthias van de Meent wrote:
> As I understand it, Andres agrees that the feature is unsalvageable in
> backbranches ("don't think there's a realistic way to fix it"). Andres
> then continues to elaborate that even if the feature were salvageable,
> it wouldn't co
On Sun, 16 Mar 2025 at 13:55, vignesh C wrote:
>
> On Wed, 5 Mar 2025 at 16:43, Matthias van de Meent
> wrote:
> >
> > On Sun, 2 Mar 2025 at 01:35, Tom Lane wrote:
> > >
> > > Peter Geoghegan writes:
> > > > Is everybody in agreement about committing and back patching this fix,
> > > > which si
On Wed, 5 Mar 2025 at 16:43, Matthias van de Meent
wrote:
>
> On Sun, 2 Mar 2025 at 01:35, Tom Lane wrote:
> >
> > Peter Geoghegan writes:
> > > Is everybody in agreement about committing and back patching this fix,
> > > which simply disables the optimization altogether?
> > > I myself don't se
Hi,
On 2025-03-01 19:35:15 -0500, Tom Lane wrote:
> Peter Geoghegan writes:
> > Is everybody in agreement about committing and back patching this fix,
> > which simply disables the optimization altogether?
> > I myself don't see a better way, but thought I'd ask before proceeding
> > with review
On Sun, 2 Mar 2025 at 01:35, Tom Lane wrote:
>
> Peter Geoghegan writes:
> > Is everybody in agreement about committing and back patching this fix,
> > which simply disables the optimization altogether?
> > I myself don't see a better way, but thought I'd ask before proceeding
> > with review and
Peter Geoghegan writes:
> Is everybody in agreement about committing and back patching this fix,
> which simply disables the optimization altogether?
> I myself don't see a better way, but thought I'd ask before proceeding
> with review and commit.
If you don't see a clear path forward, then "dis
On Fri, Feb 28, 2025 at 12:53 PM Matthias van de Meent
wrote:
> Rebased again, now on top of head due to f7a8fc10.
Is everybody in agreement about committing and back patching this fix,
which simply disables the optimization altogether?
I myself don't see a better way, but thought I'd ask before
On Tue, 7 Jan 2025 at 18:46, Matthias van de Meent
wrote:
>
> Hi,
>
> I've rebased my earlier patch to fix some minor conflicts with the
> work done on bitmaps in December last year. I've also included Andres'
> cursor-based isolation test as 0002; which now passes.
Rebased again, now on top of h
Hi,
I've rebased my earlier patch to fix some minor conflicts with the
work done on bitmaps in December last year. I've also included Andres'
cursor-based isolation test as 0002; which now passes.
This should take care of cfbot's misidentification of which patch to
test, and thus get CFBot to suc
On Mon, 2 Dec 2024 at 17:31, Andres Freund wrote:
>
> Hi,
>
> On 2024-12-02 16:08:02 +0100, Matthias van de Meent wrote:
> > Concurrency timeline:
> >
> > Session 1. amgetbitmap() gets snapshot of index contents, containing
> > references to dead tuples on heap P1.
>
> IIUC, an unstanted important
On Mon, 2 Dec 2024 at 18:02, Tom Lane wrote:
>
> Andres Freund writes:
> > I think the problematic scenario involves tuples that *nobody* can see.
> > During
> > the bitmap index scan we don't know that though. Thus the tid gets inserted
> > into the bitmap. Then, before we visit the heap, a con
On Mon, 2 Dec 2024 at 15:25, Konstantin Knizhnik wrote:
>
> Hi hackers,
>
> Attached script reproduces the problem with incorrect results of `select
> count(*)` (it returns larger number of records than really available in the
> table).
> It is not always reproduced, so you may need to repeat it
Peter Geoghegan writes:
> On Mon, Dec 2, 2024 at 10:15 AM Matthias van de Meent
> wrote:
>> The running theory is that bitmap executor nodes incorrectly assume
>> that the rows contained in the bitmap all are still present in the
>> index, and thus assume they're allowed to only check the visibil
Hi,
On 2024-12-02 11:07:38 -0500, Peter Geoghegan wrote:
> Clearly it would be wildly impractical to do the "buffer pin interlock
> against TID recycling" thing in the context of bitmap scans.
That's certainly true if we do the VM check at the time of the bitmap heap
scan.
What if we did it when
On Mon, Dec 2, 2024 at 3:56 PM Peter Geoghegan wrote:
> I took what you wrote, and repurposed it to prove my old theory about
> GiST index-only scans being broken due to the lack of an appropriate
> interlock against concurrent TID recycling. See the attached patch.
I've moved discussion of this
On Mon, Dec 2, 2024 at 12:15 PM Peter Geoghegan wrote:
> The underlying reason why nbtree can discriminate like this is that it
> "knows" that plain index scans will always visit the heap proper. If a
> TID points to an LP_UNUSED item, then it is considered dead to the
> scan (even though in gener
Andres Freund writes:
> I think the problematic scenario involves tuples that *nobody* can see. During
> the bitmap index scan we don't know that though. Thus the tid gets inserted
> into the bitmap. Then, before we visit the heap, a concurrent vacuum removes
> the tuple from the indexes and then
On Mon, Dec 2, 2024 at 12:02 PM Tom Lane wrote:
> Andres Freund writes:
> > I think the problematic scenario involves tuples that *nobody* can see.
> > During
> > the bitmap index scan we don't know that though. Thus the tid gets inserted
> > into the bitmap. Then, before we visit the heap, a co
Peter Geoghegan writes:
> On Mon, Dec 2, 2024 at 12:02 PM Tom Lane wrote:
>> Yup. I am saying that that qualifies as too-aggressive setting of the
>> all-visible bit. I'm not sure what rule we should adopt instead of
>> the current one, but I'd much rather slow down page freezing than
>> instit
On Mon, Dec 2, 2024 at 11:32 AM Andres Freund wrote:
> On 2024-12-02 16:08:02 +0100, Matthias van de Meent wrote:
> > Concurrency timeline:
> >
> > Session 1. amgetbitmap() gets snapshot of index contents, containing
> > references to dead tuples on heap P1.
>
> IIUC, an unstanted important aspect
Hi hackers,
Attached script reproduces the problem with incorrect results of `select
count(*)` (it returns larger number of records than really available in
the table).
It is not always reproduced, so you may need to repeat it multiple times
- at my system it failed 3 times from 10.
The prob
Hi,
On 2024-12-02 11:43:42 -0500, Tom Lane wrote:
> Peter Geoghegan writes:
> > This theory seems very believable.
>
> I'm not convinced. I think there are two assumptions underlying
> bitmap scan:
>
> 1. Regardless of index contents, it's not okay for vacuum to remove
> tuples that an open tr
On Mon, Dec 2, 2024 at 3:55 PM Andres Freund wrote:
> I suspect one contributor to this avoiding attention till now was that the
> optimization is fairly narrow:
>
> /*
> * We can potentially skip fetching heap pages if we
> do not need
>
On Mon, Dec 2, 2024 at 3:56 PM Peter Geoghegan wrote:
> I took what you wrote, and repurposed it to prove my old theory about
> GiST index-only scans being broken due to the lack of an appropriate
> interlock against concurrent TID recycling. See the attached patch.
BTW, if you change the test ca
On Mon, Dec 2, 2024 at 2:43 PM Andres Freund wrote:
> Attached is an isolationtest that reliably shows wrong query results.
Nice approach with the cursor!
I took what you wrote, and repurposed it to prove my old theory about
GiST index-only scans being broken due to the lack of an appropriate
in
Hi,
On 2024-12-02 13:39:43 -0500, Peter Geoghegan wrote:
> I guess it's natural to suspect more recent work -- commit 7c70996e is
> about 6 years old. But I the race condition that I suspect is at play
> here is very narrow.
FWIW, the test I just posted shows the issue down to 11 (although for 11
On Mon, Dec 2, 2024 at 11:52 AM Andres Freund wrote:
> I think the problematic scenario involves tuples that *nobody* can see. During
> the bitmap index scan we don't know that though.
Right, exactly.
FWIW, this same issue is why it is safe for nbtree to drop its pin
early during plain index sca
Hi,
On 2024-12-02 12:02:39 -0500, Tom Lane wrote:
> Andres Freund writes:
> > I think the problematic scenario involves tuples that *nobody* can see.
> > During
> > the bitmap index scan we don't know that though. Thus the tid gets inserted
> > into the bitmap. Then, before we visit the heap, a
On Mon, Dec 2, 2024 at 11:43 AM Tom Lane wrote:
> Whether that's true or not, it seems like it'd be worth bisecting
> to see if we can finger a commit where the behavior changed (and
> the same goes for the question of why-isnt-it-an-IOS-scan). However,
> the reproducer seems to have quite a low
Hi,
On 2024-12-02 11:31:48 -0500, Andres Freund wrote:
> I think it'd be good if we added a test that shows the failure mechanism so
> that we don't re-introduce this in the future. Evidently this failure isn't
> immediately obvious...
Attached is an isolationtest that reliably shows wrong query
Hi,
On 2024-12-02 16:08:02 +0100, Matthias van de Meent wrote:
> Concurrency timeline:
>
> Session 1. amgetbitmap() gets snapshot of index contents, containing
> references to dead tuples on heap P1.
IIUC, an unstanted important aspect here is that these dead tuples are *not*
visible to S1. Othe
On Mon, Dec 2, 2024 at 12:11 PM Tom Lane wrote:
> > Freezing a page, and setting a page all-visible are orthogonal.
>
> Sorry, sloppy wording on my part.
Freezing doesn't affect the contents of the visibility map in any way
that seems relevant. The executor only cares about the all-visible bit
(a
On Mon, Dec 2, 2024 at 10:15 AM Matthias van de Meent
wrote:
> The running theory is that bitmap executor nodes incorrectly assume
> that the rows contained in the bitmap all are still present in the
> index, and thus assume they're allowed to only check the visibility
> map to see if the referenc
40 matches
Mail list logo
