Hi, On 2020-04-29 11:28:00 -0700, Peter Geoghegan wrote: > On Thu, Apr 16, 2020 at 11:27 AM Andres Freund <and...@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.
Well, you'd need to have a workload that has bulk deletes, high xid usage *and* doesn't insert new data to use those empty pages > 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. I think it's quite foolish for btvacuumpage() to not freeze xids. If we only do so when necessary (i.e. older than a potential new relfrozenxid, and only when the vacuum didn't yet skip pages), the costs are pretty miniscule. > 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. I'd much rather make sure the xid is guaranteed to be removed. As outlined above, the cost would be small, and I think the likelihood of the consequences of wrapped around xids getting worse over time is substantial. > 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?). I'd just WAL log the reuse when freezing the xid. Then there's no worry about wraparounds. And I don't think it'd cause additional conflicts; the vacuum itself (or a prior vacuum) would also have to cause them, I think? Greetings, Andres Freund
