Re: visibility map corruption

On Tue, Jul 6, 2021 at 11:57 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Peter Geoghegan <pg(at)bowt(dot)ie> writes:
> > ... We should just carry forward the original oldestXid.
>
> Yup. It's a bit silly that we recognized the need to do that
> for oldestMultiXid yet not for oldestXid.

True. But at the same time it somehow doesn't seem silly at all. IME
some of the most devious bugs evade detection by hiding in plain
sight.

It looks like amcheck's verify_heapam.c functionality almost catches
bugs like this one. Something for Mark (CC'd) to consider. Does it
matter that we usually "ctx.oldest_xid = ctx.relfrozenxid", and so
usually use pg_class.relfrozenxid as our oldest_xid (and not
ShmemVariableCache->oldestXid)? In other words, could we be doing more
to sanitize ShmemVariableCache->oldestXid, especially when the
relation's pg_class.relfrozenxid happens to be set to a real XID?

> BTW, is it really necessary for copy_xact_xlog_xid to invoke pg_resetwal
> so many times? Why can't we pass all of the update-this options in one
> call?

No opinion here.

> Who's going to do the legwork on this?

Can Bruce take care of committing the patch for this? Bruce?

This should definitely be in the next point release IMV.

--
Peter Geoghegan