On Thu, Apr 16, 2020 at 11:27 AM Andres Freund <andres@anarazel.de> wrote:
> Sure, there is some pre-existing wraparound danger for individual
> pages. But it's a pretty narrow corner case before INDEX_CLEANUP
> off.
>
> That comment says something about "shared-memory free space map", making
> it sound like any crash would loose the page. But it's a normal FSM
> these days. Vacuum will insert the deleted page into the free space
> map. So either the FSM would need to be corrupted to not find the
> inserted page anymore, or the index would need to grow slow enough to
> not use a page before the wraparound. And then such wrapped around xids
> would exist on individual pages. Not on all deleted pages, like with
> INDEX_CLEANUP false.
Is that really that narrow, even without "INDEX_CLEANUP false"? It's
not as if the index needs to grow very slowly to have only very few
page splits hour to hour (it depends on whether the inserts are random
or not, and so on). Especially if you had a bulk DELETE affecting many
rows, which is hardly that uncommon.
Fundamentally, btvacuumpage() doesn't freeze 32-bit XIDs (from
bpto.xact) when it recycles deleted pages. It simply puts them in the
FSM without changing anything about the page itself. This means
surprisingly little in the context of nbtree: the
_bt_page_recyclable() XID check that takes place in btvacuumpage()
also takes place in _bt_getbuf(), at the point where the page actually
gets recycled by the client. That's not great.
It wouldn't be so unreasonable if btvacuumpage() actually did freeze
the bpto.xact value at the point where it puts the page in the FSM. It
doesn't need to be crash safe; it can work as a hint. Maybe "freezing"
is the wrong word (too much baggage). More like we'd have VACUUM
represent that "this deleted B-Tree page is definitely not considered
to still be a part of the tree by any possible other backend" using a
page flag hint -- btvacuumpage() would "mark the deleted page as
recyclable" explicitly. Note that we still need to keep the original
bpto.xact XID around for _bt_log_reuse_page() (also, do we need to
worry _bt_log_reuse_page() with a wrapped-around XID?).
--
Peter Geoghegan
