Horton

Thread

Commits

  1. Standardize cleanup lock terminology.

  2. Reduce pinning and buffer content locking for btree scans.

  1. Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Peter Geoghegan <pg@bowt.ie> — 2021-11-03T23:33:08Z

    The code in gistvacuum.c is closely based on similar code in nbtree.c,
except that it only acquires an exclusive lock -- not a
super-exclusive lock. I suspect that that's because it seemed
unnecessary; nbtree plain index scans have their own special reasons
for this, that don't apply to GiST. Namely: nbtree plain index scans
that don't use an MVCC snapshot clearly need some other interlock to
protect against concurrent recycling of pointed-to-by-leaf-page TIDs.
And so as a general rule nbtree VACUUM needs a full
super-exclusive/cleanup lock, just in case there is a plain index scan
that uses some other kind of snapshot (logical replication, say).

To say the same thing another way: nbtree follows "the third rule"
described by "62.4. Index Locking Considerations" in the docs [1], but
GiST does not. The idea that GiST's behavior is okay here does seem
consistent with what the same docs go on to say about it: "When using
an MVCC-compliant snapshot, there is no problem because the new
occupant of the slot is certain to be too new to pass the snapshot
test".

But what about index-only scans, which GiST also supports? I think
that the rules are different there, even though index-only scans use
an MVCC snapshot.

The (admittedly undocumented) reason why we can never drop the leaf
page pin for an index-only scan in nbtree (but can do so for plain
index scans) also relates to heap interlocking -- but with a twist.
Here's the twist: the second heap pass by VACUUM can set visibility
map bits independently of the first (once LP_DEAD items from the first
pass over the heap are set to LP_UNUSED, which renders the page
all-visible) -- this all happens at the end of
lazy_vacuum_heap_page(). That's why index-only scans can't just assume
that VACUUM won't have deleted the TID from the leaf page they're
scanning immediately after they're done reading it. VACUUM could even
manage to set the visibility map bit for a relevant heap page inside
lazy_vacuum_heap_page(), before the index-only scan can read the
visibility map. If that is allowed to happen, the index-only would
give wrong answers if one of the TID references held in local memory
by the index-only scan happens to be marked LP_UNUSED inside
lazy_vacuum_heap_page(). IOW, it looks like we run the risk of a
concurrently recycled dead-to-everybody TID becoming visible during
GiST index-only scans, just because we have no interlock.

In summary:

UUIC this is only safe for nbtree because 1.) It acquires a
super-exclusive lock when vacuuming leaf pages, and 2.) Index-only
scans never drop their pin on the leaf page when accessing the
visibility map "in-sync" with the scan (of course we hope not to
access the heap proper at all for index-only scans). These precautions
are both necessary to make the race condition I describe impossible,
because they ensure that VACUUM cannot reach lazy_vacuum_heap_page()
until after our index-only scan reads the visibility map (and then has
to read the heap for at least that one dead-to-all TID, discovering
that the TID is dead to its snapshot). Why wouldn't GiST need to take
the same precautions, though?

[1] https://www.postgresql.org/docs/devel/index-locking.html
--
Peter Geoghegan

  2. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    x4mmm@yandex-team.ru — 2021-11-04T15:52:22Z

    <div> </div><div> </div><div>04.11.2021, 04:33, "Peter Geoghegan" &lt;pg@bowt.ie&gt;:</div><blockquote><p>But what about index-only scans, which GiST also supports? I think<br />that the rules are different there, even though index-only scans use<br />an MVCC snapshot.</p></blockquote><p>Let's enumerate steps how things can go wrong.</p><p>Backend1: Index-Only scan returns tid and xs_hitup with index_tuple1 on index_page1 pointing to heap_tuple1 on page1</p><p>Backend2: Remove index_tuple1 and heap_tuple1</p><div><div>Backend3: Mark page1 all-visible</div><div>Backend1: Thinks that page1 is all-visible and shows index_tuple1 as visible</div><div> </div><div>To avoid this Backend1 must hold pin on index_page1 until it's done with checking visibility, and Backend2 must do LockBufferForCleanup(index_page1). Do I get things right?</div><div> </div><div>Best regards, Andrey Borodin.</div></div><div> </div>

  3. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Peter Geoghegan <pg@bowt.ie> — 2021-11-04T15:58:29Z

    On Thu, Nov 4, 2021 at 8:52 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
> Let's enumerate steps how things can go wrong.
>
> Backend1: Index-Only scan returns tid and xs_hitup with index_tuple1 on index_page1 pointing to heap_tuple1 on page1
>
> Backend2: Remove index_tuple1 and heap_tuple1
>
> Backend3: Mark page1 all-visible
> Backend1: Thinks that page1 is all-visible and shows index_tuple1 as visible
>
> To avoid this Backend1 must hold pin on index_page1 until it's done with checking visibility, and Backend2 must do LockBufferForCleanup(index_page1). Do I get things right?

Almost. Backend3 is actually Backend2 here (there is no 3) -- it runs
VACUUM throughout.

Note that it's not particularly likely that Backend2/VACUUM will "win"
this race, because it typically has to do much more work than
Backend1. It has to actually remove the index tuples from the leaf
page, then any other index work (for this and other indexes). Then it
has to arrive back in vacuumlazy.c to set the VM bit in
lazy_vacuum_heap_page(). That's a pretty unlikely scenario. And even
if it happened it would only happen once (until the next time we get
unlucky). What are the chances of somebody noticing a more or less
once-off, slightly wrong answer?

-- 
Peter Geoghegan

  4. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    x4mmm@yandex-team.ru — 2021-11-05T10:26:09Z

    
> 4 нояб. 2021 г., в 20:58, Peter Geoghegan <pg@bowt.ie> написал(а):
> That's a pretty unlikely scenario. And even
> if it happened it would only happen once (until the next time we get
> unlucky). What are the chances of somebody noticing a more or less
> once-off, slightly wrong answer?

I'd say next to impossible, yet not impossible. Or, perhaps, I do not see protection from this.

Moreover there's a "microvacuum". It kills tuples with BUFFER_LOCK_SHARE. AFAIU it should take cleanup lock on buffer too?

Best regards, Andrey Borodin.

  5. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Peter Geoghegan <pg@bowt.ie> — 2021-12-01T01:09:14Z

    On Fri, Nov 5, 2021 at 3:26 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
> > 4 нояб. 2021 г., в 20:58, Peter Geoghegan <pg@bowt.ie> написал(а):
> > That's a pretty unlikely scenario. And even
> > if it happened it would only happen once (until the next time we get
> > unlucky). What are the chances of somebody noticing a more or less
> > once-off, slightly wrong answer?
>
> I'd say next to impossible, yet not impossible. Or, perhaps, I do not see protection from this.

I think that that's probably all correct -- I would certainly make the
same guess. It's very unlikely to happen, and when it does happen it
happens only once.

> Moreover there's a "microvacuum". It kills tuples with BUFFER_LOCK_SHARE. AFAIU it should take cleanup lock on buffer too?

No, because there is no heap vacuuming involved (because that doesn't
happen outside lazyvacuum.c). The work that VACUUM does inside
lazy_vacuum_heap_rel() is part of the problem here -- we need an
interlock between that work, and index-only scans. Making LP_DEAD
items in heap pages LP_UNUSED is only ever possible during a VACUUM
operation (I'm sure you know why). AFAICT there would be no bug at all
without that detail.

I believe that there have been several historic reasons why we need a
cleanup lock during nbtree VACUUM, and that there is only one
remaining reason for it today. So the history is unusually complicated. But
AFAICT it's always some kind of "interlock with heapam VACUUM" issue,
with TID recycling, with no protection from our MVCC snapshot. I would
say that that's the "real problem" here, when I get to first principles.

Attached draft patch attempts to explain things in this area within
the nbtree README. There is a much shorter comment about it within
vacuumlazy.c. I am concerned about GiST index-only scans themselves,
of course, but I discovered this issue when thinking carefully about
the concurrency rules for VACUUM -- I think it's valuable to formalize
and justify the general rules that index access methods must follow.

We can talk about this some more in NYC. See you soon!
--
Peter Geoghegan

  6. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Peter Geoghegan <pg@bowt.ie> — 2021-12-01T01:39:19Z

    On Tue, Nov 30, 2021 at 5:09 PM Peter Geoghegan <pg@bowt.ie> wrote:
> I believe that there have been several historic reasons why we need a
> cleanup lock during nbtree VACUUM, and that there is only one
> remaining reason for it today. So the history is unusually complicated.

Minor correction: we actually also have to worry about plain index
scans that don't use an MVCC snapshot, which is possible within
nbtree. It's quite likely when using logical replication, actually.
See the patch for more.

Like with the index-only scan case, a non-MVCC snapshot + plain nbtree
index scan cannot rely on heap access within the index scan node -- it
won't reliably notice that any newer heap tuples (that are really the
result of concurrent TID recycling) are not actually visible to its
MVCC snapshot -- because there isn't an MVCC snapshot. The only
difference in the index-only scan scenario is that we use the
visibility map (not the heap) -- which is racey in a way that makes
our MVCC snapshot (IOSs always have an MVCC snapshot) an ineffective
protection.

In summary, to be safe against confusion from concurrent TID recycling
during index/index-only scans, we can do either of the following
things:

1. Hold a pin of our leaf page while accessing the heap -- that'll
definitely conflict with the cleanup lock that nbtree VACUUM will
inevitably try to acquire on our leaf page.

OR:

2. Hold an MVCC snapshot, AND do an actual heap page access during the
plain index scan -- do both together.

With approach 2, our plain index scan must determine visibility using
real XIDs (against something like a dirty snapshot), rather than using
a visibility map bit. That is also necessary because the VM might
become invalid or ambiguous, in a way that's clearly not possible when
looking at full heap tuple headers with XIDs -- concurrent recycling
becomes safe if we know that we'll reliably notice it and not give
wrong answers.

Does that make sense?

-- 
Peter Geoghegan

  7. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Peter Geoghegan <pg@bowt.ie> — 2021-12-09T01:40:37Z

    On Tue, Nov 30, 2021 at 5:09 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Attached draft patch attempts to explain things in this area within
> the nbtree README. There is a much shorter comment about it within
> vacuumlazy.c. I am concerned about GiST index-only scans themselves,
> of course, but I discovered this issue when thinking carefully about
> the concurrency rules for VACUUM -- I think it's valuable to formalize
> and justify the general rules that index access methods must follow.

I pushed a commit that described how this works for nbtree, in the README file.

I think that there might be an even more subtle race condition in
nbtree itself, though, during recovery. We no longer do a "pin scan"
during recovery these days (see commits 9f83468b, 3e4b7d87, and
687f2cd7 for full information). I think that it might be necessary to
do that, just for the benefit of index-only scans -- if it's necessary
during original execution, then why not during recovery?

The work to remove "pin scans" was justified by pointing out that we
no longer use various kinds of snapshots during recovery, but it said
nothing about index-only scans, which need the TID recycling interlock
(i.e. need to hold onto a leaf page while accessing the heap in sync)
even with an MVCC snapshot. It's easy to imagine how it might have
been missed: nobody ever documented the general issue with index-only
scans, until now. Commit 2ed5b87f recognized they were unsafe for the
optimization that it added (to avoid blocking VACUUM), but never
explained why they were unsafe.

Going back to doing pin scans during recovery seems deeply
unappealing, especially to fix a totally narrow race condition.

-- 
Peter Geoghegan

  8. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Peter Geoghegan <pg@bowt.ie> — 2024-12-03T01:18:14Z

    On Wed, Nov 3, 2021 at 7:33 PM Peter Geoghegan <pg@bowt.ie> wrote:
> But what about index-only scans, which GiST also supports? I think
> that the rules are different there, even though index-only scans use
> an MVCC snapshot.

(Reviving this old thread after 3 years)

I was reminded of this old thread during today's discussion of a
tangentially related TID-recycle-safety bug that affects bitmap index
scans that use the visibility map as an optimization [1]. Turns out I
was right to be concerned.

This GiST bug is causally unrelated to that other bug, so I thought it
would be helpful to move discussion of the GiST bug to this old
thread.

Attached is a refined version of a test case I posted earlier on [2],
decisively proving that GiST index-only scans are in fact subtly
broken. Right now it fails, showing a wrong answer to a query. The
patch adds an isolationtest test case to btree_gist, based on a test
case of Andres'.

Offhand, I think that the only way to fix this is to bring GiST in
line with nbtree: we ought to teach GiST VACUUM to start acquiring
cleanup locks (previously known as super-exclusive locks), and then
teach GiST index-only scans to hold onto a leaf page buffer pin as the
visibility map (or the heap proper) is accessed for the TIDs to be
returned from the leaf page. Arguably, GiST isn't obeying the current
contract for amgettuple index AMs at all right now (whether or not it
violates the contract as written is open to interpretation, I suppose,
but either way the current behavior is wrong).

We probably shouldn't hold onto a buffer pin during plain GiST
index-only scans, though -- plain GiST index scans *aren't* broken,
and so we should change as little as possible there. More concretely,
we should probably copy more nbtree scan related code into GiST to
deal with all this: we could copy nbtree's _bt_drop_lock_and_maybe_pin
into GiST to fix this bug, while avoiding changing the performance
characteristics of GiST plain index scans. This will also entail
adding a new buffer field to GiST's GISTScanOpaqueData struct --
something similar to nbtree's BTScanOpaqueData.currPos.buf field
(it'll go next to the current GISTScanOpaqueData.curBlkno field, just
like the nbtree equivalent goes next to its own currPage blkno field).

Long term, code like nbtree's _bt_drop_lock_and_maybe_pin should be
generalized and removed from every individual index AM, nbtree
included -- I think that the concepts generalize to every index AM
that supports amgettuple (the race condition in question has
essentially nothing to do with individual index AM requirements). I've
discussed this kind of approach with Tomas Vondra (CC'd) recently.
That's not going to be possible within the scope of a backpatchable
fix, though.

[1] https://postgr.es/m/873c33c5-ef9e-41f6-80b2-2f5e11869f1c@garret.ru
[2] https://postgr.es/m/CAH2-Wzm6gBqc99iEKO6540ynwpjOqWESt5yjg-bHbt0hc8DPsw%40mail.gmail.com
--
Peter Geoghegan

  9. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Peter Geoghegan <pg@bowt.ie> — 2024-12-03T16:21:04Z

    On Mon, Dec 2, 2024 at 8:18 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Attached is a refined version of a test case I posted earlier on [2],
> decisively proving that GiST index-only scans are in fact subtly
> broken. Right now it fails, showing a wrong answer to a query. The
> patch adds an isolationtest test case to btree_gist, based on a test
> case of Andres'.

I can confirm that the same bug affects SP-GiST. I modified the
original failing GiST isolation test to make it use SP-GiST instead,
proving what I already strongly suspected.

I have no reason to believe that there are any similar problems in
core index AMs other than GiST and SP-GiST, though. Let's go through
them all now: nbtree already does everything correctly, and all
remaining core index AMs don't support index-only scans *and* don't
support scans that don't just use an MVCC snapshot.

-- 
Peter Geoghegan

  10. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-01-04T01:00:34Z

    On Tue, 3 Dec 2024 at 17:21, Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Mon, Dec 2, 2024 at 8:18 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > Attached is a refined version of a test case I posted earlier on [2],
> > decisively proving that GiST index-only scans are in fact subtly
> > broken. Right now it fails, showing a wrong answer to a query. The
> > patch adds an isolationtest test case to btree_gist, based on a test
> > case of Andres'.
>
> I can confirm that the same bug affects SP-GiST. I modified the
> original failing GiST isolation test to make it use SP-GiST instead,
> proving what I already strongly suspected.
>
> I have no reason to believe that there are any similar problems in
> core index AMs other than GiST and SP-GiST, though. Let's go through
> them all now: nbtree already does everything correctly, and all
> remaining core index AMs don't support index-only scans *and* don't
> support scans that don't just use an MVCC snapshot.

I think I have a fix for GiST which can be found attached in patch 0002.

As to how this works: the patch tracks (for IOS) the pages for which
there are some entries yet to be returned by gistgettuple(), and keeps
a pin on those pages using a new AMscan tracking mechanism that
utilizes buffer refcounts. Even if it might not a very elegant
solution and IMV still has rough edges, it works, and fixes the issue
with incorrect results from the GiST index.

One side effect of this change to keep pins in GiST-IOS, is that that
this could realistically keep pins on a huge portion of the index,
thus exhausting shared buffers and increasing prevalence of "no
unpinned buffers"-related errors.

I haven't looked much at SP-GiST yet, so I don't have anything for the
VACUUM+IOS bug there.

0001 is a slight modification of your v2-0001, a version which now
(critically) doesn't expect VACUUM to run to completion before
s1_fetch_all starts; this is important for 0002 as that causes vacuum
to block and wait for the cursor to return more tuples, which the
isolation tester doesn't (can't?) detect. With only 0001, the new test
fails with incorrect results, with 0002 applied the test succeeds.

I'm looking forward to any feedback.

Kind regards,

Matthias van de Meent

  11. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-01-06T23:45:18Z

    On Sat, 4 Jan 2025 at 02:00, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Tue, 3 Dec 2024 at 17:21, Peter Geoghegan <pg@bowt.ie> wrote:
> >
> > On Mon, Dec 2, 2024 at 8:18 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > > Attached is a refined version of a test case I posted earlier on [2],
> > > decisively proving that GiST index-only scans are in fact subtly
> > > broken. Right now it fails, showing a wrong answer to a query. The
> > > patch adds an isolationtest test case to btree_gist, based on a test
> > > case of Andres'.
> >
> > I can confirm that the same bug affects SP-GiST. I modified the
> > original failing GiST isolation test to make it use SP-GiST instead,
> > proving what I already strongly suspected.
> >
> > I have no reason to believe that there are any similar problems in
> > core index AMs other than GiST and SP-GiST, though. Let's go through
> > them all now: nbtree already does everything correctly, and all
> > remaining core index AMs don't support index-only scans *and* don't
> > support scans that don't just use an MVCC snapshot.
>
> I think I have a fix for GiST which can be found attached in patch 0002.
>
> As to how this works: the patch tracks (for IOS) the pages for which
> there are some entries yet to be returned by gistgettuple(), and keeps
> a pin on those pages using a new AMscan tracking mechanism that
> utilizes buffer refcounts. Even if it might not a very elegant
> solution and IMV still has rough edges, it works, and fixes the issue
> with incorrect results from the GiST index.

In the attached v4 of the fix I've opted to replace the bespoke pin
tracker of my previous fix with the default buffer pin tracking
mechanism, which I realised has been designed for the same items and
doesn't require additional memory management on the AM side.
It massively simplifies the code, reduces allocation overhead, and
allowed me to port the fix to SP-GiST much quicker.

> I haven't looked much at SP-GiST yet, so I don't have anything for the
> VACUUM+IOS bug there.

I've attached a fix that uses the same approach as GiST in v4-0003. I
couldn't find any spgist extensions in contrib to copy-paste the tests
to, but manual testing did show vacuum now does wait for the index
scan to finish page processing.

> I'm looking forward to any feedback.

Like patch 0001, the status of that has not changed.

Kind regards,

Matthias van de Meent

  12. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Michail Nikolaev <michail.nikolaev@gmail.com> — 2025-01-09T21:00:03Z

    Hello.

One thing I think we could add to the patches is to adapt the 10-years-old
comment below with notice about IOS:

/*
* We save the LSN of the page as we read it, so that we know whether it
* safe to apply LP_DEAD hints to the page later. This allows us to drop
* the pin for MVCC scans, which allows vacuum to avoid blocking.
*/
so->curPageLSN = BufferGetLSNAtomic(buffer);


Also, I think it is a good idea to add "Assert(!scan->xs_want_itup);"
to gistkillitems.

Best regards,
Mikhail.

  13. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-01-09T21:35:17Z

    On Thu, 9 Jan 2025 at 22:00, Michail Nikolaev
<michail.nikolaev@gmail.com> wrote:
>
> Hello.
>
> One thing I think we could add to the patches is to adapt the 10-years-old comment below with notice about IOS:
>
> /*
> * We save the LSN of the page as we read it, so that we know whether it
> * safe to apply LP_DEAD hints to the page later. This allows us to drop
> * the pin for MVCC scans, which allows vacuum to avoid blocking.
> */
> so->curPageLSN = BufferGetLSNAtomic(buffer);

I don't quite read it as covering IOS. To me, the comment is more
along the lines of (extensively extended):

"""
We'd like to use kill_prior_tuple, but that requires us to apply
changes to the page when we're already done with it for all intents
and purposes (because we scan the page once and buffer results). We
can either keep a pin on the buffer, or re-acquire that page after
finishing producing the tuples from this page.
Pinning the page blocks vacuum [^1], so instead we drop the pin, then
collect LP_DEAD marks, and then later we re-acquire the page to mark
the tuples dead. However, in the meantime the page may have changed;
by keeping a tab on changes in LSN we have a cheap method of detecting
changes in the page iself. [^2]
"""

... and that doesn't seem to cover much of IOS. MVCC index scans
aren't that special, practically every user query has MVCC. I think
this "MVCC scan" even means non-IOS scan, as I can't think of a reason
why dropping a pin would otherwise be a valid behaviour (see the this
thread's main issue).

[^1] Well, it should. In practice, the code in HEAD doesn't, but this
patchset fixes that disagreement.
[^2] If the page changed, i.e. the LSN changed, GIST accepts that it
can't use the collected LP_DEAD marks. We may be able to improve on
that (or not) by matching LP_DEAD offsets and TIDs with the on-page
TIDs, but that's far outside the scope of this patch; we'd first have
to build an understanding about why it's correct to assume vacuum
hasn't finished reaping old tuples and other sessions also finished
inserting new ones with the same TIDs in the meantime.

> Also, I think it is a good idea to add "Assert(!scan->xs_want_itup);" to gistkillitems.

Why would it be incorrect or invalid to kill items in an index-only scan?
If we hit the heap (due to ! VM_ALL_VISIBLE) and detected the heap
tuple was dead, why couldn't we mark it as dead in the index? IOS
assumes a high rate of all-visible pages, but it's hardly unheard of
to access pages with dead tuples.

Kind regards,

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

  14. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Michail Nikolaev <michail.nikolaev@gmail.com> — 2025-01-10T01:09:42Z

    Hello!

Sorry, I should have expressed my thoughts in more detail, as they don't
matter as much as the time you took to answer.

>I don't quite read it as covering IOS. To me, the comment is more
> along the lines of (extensively extended):

My idea was just to add a few more details about the locking rule, such as:

* safe to apply LP_DEAD hints to the page later. This allows us to drop
* the pin for MVCC scans (except in cases of index-only scans due to XXX),
which allows vacuum to avoid blocking.

>   I think this "MVCC scan" even means non-IOS scan

Maybe, but I think it’s better to clarify that, since IOS scans still use
the MVCC snapshot. For me, a non-MVCC scan is something like SnapshotSelf
or SnapshotDirty.

>  Why would it be incorrect or invalid to kill items in an index-only scan?

Oh, I was comparing the logic to that of btree and somehow made a logical
error in my conclusions. But at least I hope I got some useful thoughts out
of it - since we have a pin during gistkillitems in the case of IOS, we can
ignore the "if (BufferGetLSNAtomic(buffer) != so->curPageLSN)" check in
that case because vacuum is locked.
It is not a compensation for a performance penalty caused by buffer pin
during IOS, but at least something.

I hope this time my conclusions are correct :)

Thanks,
Mikhail.

  15. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Michail Nikolaev <michail.nikolaev@gmail.com> — 2025-01-10T17:06:10Z

    Hello, Matthias!

Updated patches attached.

Changes:
* cleanup test logic a little bit
* resolve issue with rescan in GIST (item->blkno == InvalidBlockNumber)
* move test to the main isolation suite
* add test for SpGist
* update comment I mentioned before
* allow GIST to set LP_DEAD in cases it is a safe even if LSN is updated

Also, seems like SP-GIST version is broken, it fails like this:

TRAP: failed Assert("BufferIsValid(so->pagePin)"), File:
"../src/backend/access/spgist/spgscan.c", Line: 1139, PID: 612214
FETCH(ExceptionalCondition+0xbe)[0x644a0b9dfdbc]
FETCH(spggettuple+0x289)[0x644a0b3743c6]
FETCH(index_getnext_tid+0x166)[0x644a0b3382f7]
FETCH(+0x3b392b)[0x644a0b56d92b]
FETCH(+0x3887df)[0x644a0b5427df]
FETCH(ExecScan+0x77)[0x644a0b542858]
FETCH(+0x3b3b9b)[0x644a0b56db9b]
FETCH(+0x376d8b)[0x644a0b530d8b]
FETCH(+0x379bd9)[0x644a0b533bd9]
FETCH(standard_ExecutorRun+0x19f)[0x644a0b531457]
FETCH(ExecutorRun+0x5a)[0x644a0b5312b5]
FETCH(+0x6276dc)[0x644a0b7e16dc]
FETCH(+0x628936)[0x644a0b7e2936]
FETCH(PortalRunFetch+0x1a0)[0x644a0b7e229c]
FETCH(PerformPortalFetch+0x13b)[0x644a0b49d7e5]
FETCH(standard_ProcessUtility+0x5f0)[0x644a0b7e3aab]
FETCH(ProcessUtility+0x140)[0x644a0b7e34b4]
FETCH(+0x627ceb)[0x644a0b7e1ceb]
FETCH(+0x627a28)[0x644a0b7e1a28]
FETCH(PortalRun+0x273)[0x644a0b7e12bb]
FETCH(+0x61fae1)[0x644a0b7d9ae1]
FETCH(PostgresMain+0x9eb)[0x644a0b7df170]
FETCH(+0x61b3e2)[0x644a0b7d53e2]
FETCH(postmaster_child_launch+0x137)[0x644a0b6e6e2d]
FETCH(+0x53384b)[0x644a0b6ed84b]
FETCH(+0x530f31)[0x644a0b6eaf31]
FETCH(PostmasterMain+0x161f)[0x644a0b6ea812]
FETCH(main+0x3a1)[0x644a0b5c29cf]

Best regards,
Mikhail.

>

  16. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Michail Nikolaev <michail.nikolaev@gmail.com> — 2025-02-05T10:04:00Z

    Hello everyone, and Mathias!

I have fixed sp-gist related crash and a few issues in implementation.
Now it passes tests and (in my opinion) feels simpler.

I'll register that thread in commitfest to honor the bureaucracy.

Best regards,
Mikhail.

  17. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Michail Nikolaev <michail.nikolaev@gmail.com> — 2025-02-05T10:11:40Z

    Ooops, missed one commit - fixed (logic related to LP_DEAD in GIST
extracted to separate commit).

Also, commitfest entry is here - https://commitfest.postgresql.org/52/5542/

>

  18. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Michail Nikolaev <michail.nikolaev@gmail.com> — 2025-02-08T13:46:00Z

    Hello, everyone!

Just some commit messages + few cleanups.

Best regards,
Mikhail.

  19. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Peter Geoghegan <pg@bowt.ie> — 2025-02-28T01:53:19Z

    On Sat, Feb 8, 2025 at 8:47 AM Michail Nikolaev
<michail.nikolaev@gmail.com> wrote:
> Just some commit messages + few cleanups.

I'm worried about this:

+These longer pin lifetimes can cause buffer exhaustion with messages like "no
+unpinned buffers available" when the index has many pages that have similar
+ordering; but future work can figure out how to best work that out.

I think that we should have some kind of upper bound on the number of
pins that can be acquired at any one time, in order to completely
avoid these problems. Solving that problem will probably require GiST
expertise that I don't have right now.

-- 
Peter Geoghegan

  20. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Heikki Linnakangas <hlinnaka@iki.fi> — 2025-03-05T09:04:25Z

    On 28/02/2025 03:53, Peter Geoghegan wrote:
> On Sat, Feb 8, 2025 at 8:47 AM Michail Nikolaev
> <michail.nikolaev@gmail.com> wrote:
>> Just some commit messages + few cleanups.
> 
> I'm worried about this:
> 
> +These longer pin lifetimes can cause buffer exhaustion with messages like "no
> +unpinned buffers available" when the index has many pages that have similar
> +ordering; but future work can figure out how to best work that out.
> 
> I think that we should have some kind of upper bound on the number of
> pins that can be acquired at any one time, in order to completely
> avoid these problems. Solving that problem will probably require GiST
> expertise that I don't have right now.

+1. With no limit, it seems pretty easy to hold thousands of buffer pins 
with this.

The index can set IndexScanDesc->xs_recheck to indicate that the quals 
must be rechecked. Perhaps we should have a similar flag to indicate 
that the visibility must be rechecked.

Matthias's earlier patch 
(https://www.postgresql.org/message-id/CAEze2Wg1kbpo_Q1%3D9X68JRsgfkyPCk4T0QN%2BqKz10%2BFVzCAoGA%40mail.gmail.com) 
had a more complicated mechanism to track the pinned buffers. Later 
patch got rid of that, which simplified things a lot. I wonder if we 
need something like that, after all.


Here's a completely different line of attack: Instead of holding buffer 
pins for longer, what if we checked the visibility map earlier? We could 
check the visibility map already when we construct the 
GISTSearchHeapItem, and set a flag in IndexScanDesc to tell 
IndexOnlyNext() that we have already done that. IndexOnlyNext() would 
have three cases:

1. The index AM has not checked the visibility map. Check it in 
IndexOnlyNext(), and fetch the tuple if it's not set. This is what it 
always does today.
2. The index AM has checked the visibility map, and the VM bit was set. 
IndexOnlyNext() can skip the VM check and use the tuple directly.
3. The index AM has checked the visibility map, and the VM bit was not 
set. IndexOnlyNext() will fetch the tuple to check its visibility.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

  21. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-03-05T18:19:06Z

    On Wed, 5 Mar 2025 at 10:04, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> On 28/02/2025 03:53, Peter Geoghegan wrote:
> > On Sat, Feb 8, 2025 at 8:47 AM Michail Nikolaev
> > <michail.nikolaev@gmail.com> wrote:
> >> Just some commit messages + few cleanups.
> >
> > I'm worried about this:
> >
> > +These longer pin lifetimes can cause buffer exhaustion with messages like "no
> > +unpinned buffers available" when the index has many pages that have similar
> > +ordering; but future work can figure out how to best work that out.
> >
> > I think that we should have some kind of upper bound on the number of
> > pins that can be acquired at any one time, in order to completely
> > avoid these problems. Solving that problem will probably require GiST
> > expertise that I don't have right now.
>
> +1. With no limit, it seems pretty easy to hold thousands of buffer pins
> with this.
>
> The index can set IndexScanDesc->xs_recheck to indicate that the quals
> must be rechecked. Perhaps we should have a similar flag to indicate
> that the visibility must be rechecked.
>
> Matthias's earlier patch
> (https://www.postgresql.org/message-id/CAEze2Wg1kbpo_Q1%3D9X68JRsgfkyPCk4T0QN%2BqKz10%2BFVzCAoGA%40mail.gmail.com)
> had a more complicated mechanism to track the pinned buffers. Later
> patch got rid of that, which simplified things a lot. I wonder if we
> need something like that, after all.

I dropped that because it effectively duplicates the current
per-backend pin tracking system. Adding it back in will probably
complicate matters by a lot again.

> Here's a completely different line of attack: Instead of holding buffer
> pins for longer, what if we checked the visibility map earlier? We could
> check the visibility map already when we construct the
> GISTSearchHeapItem, and set a flag in IndexScanDesc to tell
> IndexOnlyNext() that we have already done that. IndexOnlyNext() would
> have three cases:

I don't like integrating a heap-specific thing like VM_ALL_VISIBLE()
to indexes, but given that IOS code already uses that exact code my
dislike is not to the point of a -1. I'd like it better if we had a
TableAM API for higher-level visibility checks (e.g.
table_tids_could_be_invisible?()) which gives us those responses
instead; dropping the requirement to maintain VM in pg's preferred
format to support efficient IOS.

I am a bit worried about even more random IO happening before we've
returned even a single tuple, but that's probably much less of an
issue than "unlimited pins".

With VM-checking in the index, we would potentially have another
benefit: By checking all tids on the page at once, we can deduplicate
and reduce the VM lookups. The gains might not be all that impressive,
but could be significant in certain hot cases.

Kind regards,

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

  22. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-03-08T02:36:31Z

    On Wed, 5 Mar 2025 at 19:19, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Wed, 5 Mar 2025 at 10:04, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> >
> > On 28/02/2025 03:53, Peter Geoghegan wrote:
> > > On Sat, Feb 8, 2025 at 8:47 AM Michail Nikolaev
> > > <michail.nikolaev@gmail.com> wrote:
> > >> Just some commit messages + few cleanups.
> > >
> > > I'm worried about this:
> > >
> > > +These longer pin lifetimes can cause buffer exhaustion with messages like "no
> > > +unpinned buffers available" when the index has many pages that have similar
> > > +ordering; but future work can figure out how to best work that out.
> > >
> > > I think that we should have some kind of upper bound on the number of
> > > pins that can be acquired at any one time, in order to completely
> > > avoid these problems. Solving that problem will probably require GiST
> > > expertise that I don't have right now.
> >
> > +1. With no limit, it seems pretty easy to hold thousands of buffer pins
> > with this.
> >
> > The index can set IndexScanDesc->xs_recheck to indicate that the quals
> > must be rechecked. Perhaps we should have a similar flag to indicate
> > that the visibility must be rechecked.

Added as xs_visrecheck in 0001.

> > Here's a completely different line of attack: Instead of holding buffer
> > pins for longer, what if we checked the visibility map earlier? We could
> > check the visibility map already when we construct the
> > GISTSearchHeapItem, and set a flag in IndexScanDesc to tell
> > IndexOnlyNext() that we have already done that. IndexOnlyNext() would
> > have three cases:
>
> I don't like integrating a heap-specific thing like VM_ALL_VISIBLE()
> to indexes, but given that IOS code already uses that exact code my
> dislike is not to the point of a -1. I'd like it better if we had a
> TableAM API for higher-level visibility checks (e.g.
> table_tids_could_be_invisible?()) which gives us those responses
> instead; dropping the requirement to maintain VM in pg's preferred
> format to support efficient IOS.

Here's a patchset that uses that approach. Naming of functions, types,
fields and arguments TBD. The patch works and passes the new
VACUUM-conflict tests, though I suspect the SP-GIST tests to have
bugs, as an intermediate version of my 0003 patch didn't trigger the
tests to fail, even though it did not hold a pin on (all) sorted
items' data when it was being checked for visibility and/or returned
from the scan.

Patch 0001 details the important changes, while 0002/0003 use this new
API to make GIST and SP-GIST's IOS work correctly when concurrent
VACUUM is/was running.
0004 is the existing patch with tests (v8-0001).

> With VM-checking in the index, we would potentially have another
> benefit: By checking all tids on the page at once, we can deduplicate
> and reduce the VM lookups. The gains might not be all that impressive,
> but could be significant in certain hot cases.

That is also included in this, but any performance impact hasn't been
tested nor validated.


Kind regards,

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

  23. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Michail Nikolaev <michail.nikolaev@gmail.com> — 2025-03-09T12:44:00Z

    Hello, Mathias!

>  though I suspect the SP-GIST tests to have
> bugs, as an intermediate version of my 0003 patch didn't trigger the
> tests to fail

It all fails on master - could you please detail what is "intermediate" in
that case? Also, I think it is a good idea to add the same type of test to
btree.

>  * XXX: In the future we should probably reorder these operations so
>  * we can apply the checks in block order, rather than index order.

I think it is already done in your patch, no?

Should we when use that mechanics for btree as well? It seems to be
straight forward and non-invasive. In such case, "Unchecked" goes away, and
it is each AM responsibility to call the check while holding the pin.

Best regards,
Mikhail.

  24. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    vignesh C <vignesh21@gmail.com> — 2025-03-16T12:58:17Z

    On Sat, 8 Mar 2025 at 08:06, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> Here's a patchset that uses that approach. Naming of functions, types,
> fields and arguments TBD. The patch works and passes the new
> VACUUM-conflict tests, though I suspect the SP-GIST tests to have
> bugs, as an intermediate version of my 0003 patch didn't trigger the
> tests to fail, even though it did not hold a pin on (all) sorted
> items' data when it was being checked for visibility and/or returned
> from the scan.
>
> Patch 0001 details the important changes, while 0002/0003 use this new
> API to make GIST and SP-GIST's IOS work correctly when concurrent
> VACUUM is/was running.
> 0004 is the existing patch with tests (v8-0001).

I noticed that Mikhail's feedback from [1] is not yet addressed. I
have changed the status of the commitfest entry to Waiting on Author,
kindly address them and update the status to Needs review.
[1] - https://www.postgresql.org/message-id/CANtu0ojz0apXnVia0reTL28eL2=__ev8aLsiH=1XfD_Z3dnkTw@mail.gmail.com

Regards,
Vignesh

  25. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-03-21T16:14:12Z

    On Sun, 16 Mar 2025 at 13:58, vignesh C <vignesh21@gmail.com> wrote:
>
> On Sat, 8 Mar 2025 at 08:06, Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
> >
> > Here's a patchset that uses that approach. Naming of functions, types,
> > fields and arguments TBD. The patch works and passes the new
> > VACUUM-conflict tests, though I suspect the SP-GIST tests to have
> > bugs, as an intermediate version of my 0003 patch didn't trigger the
> > tests to fail, even though it did not hold a pin on (all) sorted
> > items' data when it was being checked for visibility and/or returned
> > from the scan.
> >
> > Patch 0001 details the important changes, while 0002/0003 use this new
> > API to make GIST and SP-GIST's IOS work correctly when concurrent
> > VACUUM is/was running.
> > 0004 is the existing patch with tests (v8-0001).
>
> I noticed that Mikhail's feedback from [1] is not yet addressed. I
> have changed the status of the commitfest entry to Waiting on Author,
> kindly address them and update the status to Needs review.
> [1] - https://www.postgresql.org/message-id/CANtu0ojz0apXnVia0reTL28eL2=__ev8aLsiH=1XfD_Z3dnkTw@mail.gmail.com

While there has indeed been some feedback, so far I've been looking
for architectural feedback about how the bug would be solved, not per
se the names of variables, or the exact details of the comments on the
new code: I usually rather wait with polishing my patches until after
we've made sure it doesn't need a full rewrite due to architectural
issues (like what happened in the previous two iterations).

Attached is v10, which polishes the previous patches, and adds a patch
for nbtree to use the new visibility checking strategy so that it too
can release its index pages much earlier, and adds a similar
visibility check test to nbtree.


Kind regards,

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

  26. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-04-24T20:46:10Z

    On Fri, 21 Mar 2025 at 17:14, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
> Attached is v10, which polishes the previous patches, and adds a patch
> for nbtree to use the new visibility checking strategy so that it too
> can release its index pages much earlier, and adds a similar
> visibility check test to nbtree.

And here's v12. v11 (skipped) would've been a rebase, but after
finishing the rebase I noticed a severe regression in btree's IOS with
the new code, so v12 here applies some optimizations which reduce the
overhead of the new code.

Given its TableAM api changes it'd be nice to have a review on 0001,
though the additions could be rewritten to not (yet) add
TableAMRoutine.

I think patches 1, 2 and 3 are relevant to PG18 (as long as we don't
have a beta, and this is only a bit more than a bugfix). Patch 4 is
for PG19 to get btree to implement the new API, too, and patch 5
contains tests similar to the bitmap scan tests, validating that IOS
doesn't block VACUUM but still returns correct results.

I'll try to figure out a patch that's backpatchable, as alternative to
patches 2 and 3, or at least for back-patching into PG17-. That will
arrive separately, though.


Kind regards,

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

  27. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-12-22T22:23:05Z

    On Thu, 24 Apr 2025 at 22:46, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Fri, 21 Mar 2025 at 17:14, Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
> > Attached is v10, which polishes the previous patches, and adds a patch
> > for nbtree to use the new visibility checking strategy so that it too
> > can release its index pages much earlier, and adds a similar
> > visibility check test to nbtree.
>
> And here's v12. v11 (skipped) would've been a rebase, but after
> finishing the rebase I noticed a severe regression in btree's IOS with
> the new code, so v12 here applies some optimizations which reduce the
> overhead of the new code.

Here's v13, which moves the changes around a bit:

v12's 0001 is split into 3 patches (1, 3, and 4), whilst v12's 2-5
were correspondingly renumbered 5-8. Patch 0002 is an otherwise
unrelated change in pg_visibility that updates it to use the new
vectorized API, reducing overhead. So, summary of the patches:

0001: Replaces visibilitymap_get_status with a vectorized variant that
touches each VM page at most once per call; reducing buffer churn and
enabling later patches
0002: update pg_visibility to use this newly vectorized API (instead
of the current model that checks each page at a time)
0003: Adds the table_index_vischeck_tuples API, requiring a table AM
to expose VM checks through an API.
0004: Adjusts Index-Only scan infrastructure to make use of 0004,
instead of using VM_ALL_VISIBLE(). It also adds the relevant
infrastructure for enabling index AMs to provide the pre-checked
visibility status (from table_index_vischeck_tuples) to an index-only
scan.
0005: Implement VM-checks in GIST's IOS code
0006: Same, but for SP-GIST
0007: Same, but for NBTREE
0008: Add tests which validate that we still get correct results from
our queries, even when we use cursors to block results from getting
returned, and cleaning up tuples from those index pages.

A big benefit with this patch is that indexes now have no direct
reason anymore to hold back a VACUUM scan -- the visibility of tuples
can be checked at page scan time, and any shared resources can be
released before returning tuples to higher nodes.

------------------------

Summary of the problem that I'm solving here:

An index that holds dead tuples could return those dead tuples in an
Index-Only Scan (IOS) to the scan node, as the index AM itself doesn't
have any information about the visibility of the tuples that it
contains. The IOS infrastructure prevents these tuples from being
exposed by doing visibility checks against the Visibility Map (VM) and
-if necessary- the underlying heap.

This, however, depends on an invariant: VACUUM MUST NOT remove a TID
that's being returned by an index scan, at least not before before
that tuple has been checked for visibility in the VM; otherwise VACUUM
may get to clean up the dead tuple's page and mark it all-visible
before the visibility check occurs, incorrectly returning an
'all-visible' result for that dead tuple.

Btree indexes interlock with vacuum using a buffer cleanup lock and a
pin on pages it's yet to return results for; and that solution works
quite fine [^1].
This same solution sadly doesn't work for GiST and SP-GiST, as in a
worst case scenario they may have to sort the whole index in memory
before they can return the first index tuple, and holding pins on
those pages would be extremely punishing and might even cause system
crashes due to a lack of available un-pinned shared buffers.
To solve this, we implement a mechanism to allow indexes to do rough
visibility checks on TIDs; the results of which can then be passed
through the IndexScanDesc to indicate what the VM state was when the
tuple was still in the index. This enables them to make the VM check
happen just after they've scanned a page, but before they release
their pin on the page, adding the interlock with VACUUM without
requiring unreasonably large amounts of page pins.

This new mechanism is safe in MVCC snapshots, where we know that
tuples which are all-visible can't be removed while the scan is
ongoing, and where possibly-dead possibly-replaced TIDs are known to
be visibility-checked using transaction IDs, and where any new TID
would have to be inserted in a different transaction and therefore is
definitely invisible to our current transaction.

------------------------

Open items: review that this doesn't have any further issues, and
commits once this has been considered good enough.

Note:
  This patch changes TableAMRoutine and renames/changes exposed
functions, and as a result can't be backpatched as-is. I have a
separate thread over at [0] where I'm keeping track of a patchset that
is derived from this one and is focused on backpatching. That patchset
will contain patches 0004/0005/0006 and a reduced version of 0001+0003
to make it work in older branches without breaking external ABI
compatibility. I intend for the exposed table_index_vischeck_tuples()
API to remain consistent across the two patchsets.


Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

[0]: https://www.postgresql.org/message-id/flat/CAEze2WgH13m=MDST58KLo-NkZpbwBEt4xNWcgtghWBwRj3J0+A@mail.gmail.com

[^1]: Mostly fine, because it still holds VACUUM back when an
index-only scan holds a page pin and VACUUM needs to process that
page. If the index scan doesn't progress, then VACUUM can't progress
either, and that can cause vacuum to get stuck. That issue is solved
(for normal index scans) with patch 0007.

  28. Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-12-24T11:54:48Z

    On Mon, 22 Dec 2025 at 23:23, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Thu, 24 Apr 2025 at 22:46, Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
> >
> > On Fri, 21 Mar 2025 at 17:14, Matthias van de Meent
> > <boekewurm+postgres@gmail.com> wrote:
> > > Attached is v10, which polishes the previous patches, and adds a patch
> > > for nbtree to use the new visibility checking strategy so that it too
> > > can release its index pages much earlier, and adds a similar
> > > visibility check test to nbtree.
> >
> > And here's v12. v11 (skipped) would've been a rebase, but after
> > finishing the rebase I noticed a severe regression in btree's IOS with
> > the new code, so v12 here applies some optimizations which reduce the
> > overhead of the new code.
>
> Here's v13, which moves the changes around a bit:

CFBot reported failures, which appeared to be due to an oversight in
patch 0001, where visibilitymap_get_status was missing a static
modifier to accompany its inline nature.

Apart from that fix v14 is identical to v13.

Kind regards,

Matthias van de Meent