Re: Incorrect result of bitmap heap scan.

From: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Peter Geoghegan <pg(at)bowt(dot)ie>, Konstantin Knizhnik <knizhnik(at)garret(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Incorrect result of bitmap heap scan.
Date: 2024-12-04 10:39:03
Message-ID: CAEze2WjNUpU+QtThKWqo1nA2j4Mh3BDpaArXU4w6uH0XRry1gg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 2 Dec 2024 at 18:02, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Andres Freund <andres(at)anarazel(dot)de> 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 the heap and marks the page as
> > all-visible, as the deleted row version has been removed.
>
> 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
> institute new page locking rules.

I don't think it's new page locking rules, but rather a feature that
forgot to apply page locking rules after bypassing MVCC's snapshot
rules. IOS is only allowed exactly when they comply with index AM's
locking rules "only return a TID that's on a page that can't
concurrently be processed by vacuum" - why would this be different for
the bitmap equivalent?

By saying we're too aggressive with setting the all-visible bit you
seem to suggest we should add rules to vacuum that to remove LP_DEAD
items we don't just have to wait for tuples to be dead to all
transactions, but also for all transactions that might've gotten those
all_dead TIDs from indexes to have committed or rolled back, so that
no references to those TIDs exist that might consider them "possibly
visible".
We could achieve that by adding a waitpoint to vacuum (between the
index scan and the second heap scan for LP cleanup) which waits for
all concurrent transactions accessing the table to commit (thus all
bitmaps would be dropped), similar to REINDEX CONCURRENTLY's wait
phase, but that would slow down vacuum's ability to clean up old data
significantly, and change overall vacuum behaviour in a fundamental
way. I'm quite opposed to such a change.

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message jian he 2024-12-04 10:43:32 Re: NOT ENFORCED constraint feature
Previous Message Kirill Reshke 2024-12-04 10:31:47 Re: Pass ParseState as down to utility functions.