Re: new heapcheck contrib module

> On Jul 31, 2020, at 5:02 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Thu, Jul 30, 2020 at 9:38 PM Mark Dilger
> <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>>> On Jul 30, 2020, at 5:53 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> On Thu, Jul 30, 2020 at 6:10 PM Mark Dilger
>> Since I can't think of a plausible concrete example of corruption which would elicit the problem I was worrying about, I'll withdraw the argument. But that leaves me wondering about a comment that Andres made upthread:
>>
>>> On Apr 20, 2020, at 12:42 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>>
>>> I don't think random interspersed uses of CLogTruncationLock are a good
>>> idea. If you move to only checking visibility after tuple fits into
>>> [relfrozenxid, nextXid), then you don't need to take any locks here, as
>>> long as a lock against vacuum is taken (which I think this should do
>>> anyway).
>
> The version of the patch I'm looking at doesn't seem to mention
> CLogTruncationLock at all, so I'm confused about the comment. But what
> it is that you are wondering about exactly?

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 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.

Andres?

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company