Re: new heapcheck contrib module

Hi,

On 2020-07-31 08:51:50 -0700, Mark Dilger wrote:
> In earlier versions of the patch, I was guarding (perhaps
> unnecessarily) against clog truncation, (perhaps incorrectly) by
> taking the CLogTruncationLock (aka XactTruncationLock.) . I thought
> Andres was arguing that such locks were not necessary "as long as a
> lock against vacuum is taken". That's what motivated me to remove the
> clog locking business and put in the ShareUpdateExclusive lock. I
> don't want to remove the ShareUpdateExclusive lock from the patch
> without perhaps a clarification from Andres on the subject. His
> recent reply upthread seems to still support the idea that some kind
> of protection is required:

I'm not sure what I was thinking "back then", but right now I'd argue
that the best lock against vacuum isn't a SUE, but announcing the
correct ->xmin, so you can be sure that clog entries won't be yanked out
from under you. Potentially with the right flag sets to avoid old enough
tuples eing pruned.

> > I think it's not at all ok to look in the procarray or clog for xids
> > that are older than what you're announcing you may read. IOW I don't
> > think it's OK to just ignore the problem, or try to work around it by
> > holding XactTruncationLock.
>
> I don't understand that paragraph fully, in particular the part about
> "than what you're announcing you may read", since the cached value of
> relfrozenxid is not announced; we're just assuming that as long as
> vacuum cannot advance it during our scan, that we should be safe
> checking whether xids newer than that value (and not in the future)
> were committed.

With 'announcing' I mean using the normal mechanism for avoiding the
clog being truncated for values one might look up. Which is announcing
the oldest xid one may look up in PGXACT->xmin.

Greetings,

Andres Freund