Re: xid wraparound danger due to INDEX_CLEANUP false

On Sun, 28 Jun 2020 at 02:44, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> On Fri, Jun 26, 2020 at 10:15 PM Masahiko Sawada
> <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> > Regarding to the extent of the impact, this bug will affect the user
> > who turned vacuum_index_cleanup off or executed manually vacuum with
> > INDEX_CLEANUP off for a long time, after some vacuums. On the other
> > hand, the user who uses INDEX_CLEANUP off on the spot or turns
> > vacuum_index_cleanup off of the table from the start would not be
> > affected or less affected.
>
> I don't think that it's likely to cause too much trouble. It's already
> possible to leak deleted pages, if only because the FSM isn't crash
> safe. Actually, the nbtree README says this, and has since 2003:
>
> """
> (Note: if we find a deleted page with an extremely old transaction
> number, it'd be worthwhile to re-mark it with FrozenTransactionId so that
> a later xid wraparound can't cause us to think the page is unreclaimable.
> But in more normal situations this would be a waste of a disk write.)
> """
>
> But, uh, isn't the btvacuumcleanup() call supposed to avoid
> wraparound? Who knows?!
>
> It doesn't seem like the recycling aspect of page deletion was
> rigorously designed, possibly because it's harder to test than page
> deletion itself. This is a problem that we should fix.

Agreed.

>
> > I apologize for writing this patch without enough consideration. I
> > should have been more careful as I learned the nbtree page recycle
> > strategy when discussing vacuum_cleanup_index_scale_factor patch.
>
> While it's unfortunate that this was missed, let's not lose
> perspective. Anybody using the INDEX_CLEANUP feature (whether it's
> through a direct VACUUM, or by using the reloption) is already asking
> for an extreme behavior: skipping regular index vacuuming. I imagine
> that the vast majority of users that are in that position just don't
> care about the possibility of leaking deleted pages. They care about
> avoiding a real disaster from XID wraparound.

For back branches, I'm considering how we let users know about this.
For safety, we can let users know that we recommend avoiding
INDEX_CLEANUP false unless it's necessary to avoid running out of XIDs
on the documentation and/or the release note. But on the other hand,
since there is the fact that leaving recyclable pages is already
possible to happen as you mentioned I'm concerned it gets the user
into confusion and might needlessly incite unrest of users. I'm
thinking what we can do for users, in addition to leaving the summary
of this discussion as a source code comment. What do you think?

Regards,

--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services