> On Jul 30, 2020, at 1:47 PM, Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2020-07-30 13:18:01 -0700, Mark Dilger wrote:
>> Per tuple, tuple_is_visible() potentially checks whether the xmin or xmax committed via TransactionIdDidCommit. I
amworried about concurrent truncation of clog entries causing I/O errors on SLRU lookup when performing that check.
Thethree strategies I had for dealing with that were taking the XactTruncationLock (formerly known as
CLogTruncationLock,for those reading this thread from the beginning), locking out vacuum, and the idea upthread from
Andresabout setting PROC_IN_VACUUM and such. Maybe I'm being dense and don't need to worry about this. But I haven't
convincedmyself of that, yet.
>
> 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.
The current state of the patch is that concurrent vacuums are kept out of the table being checked by means of taking a
ShareUpdateExclusivelock on the table being checked. In response to Robert's review, I was contemplating whether that
wasnecessary, but you raise the interesting question of whether it is even sufficient. The logic in verify_heapam is
currentlyrelying on the ShareUpdateExclusive lock to prevent any of the xids in the range relfrozenxid..nextFullXid
frombeing invalid arguments to TransactionIdDidCommit. Ignoring whether that is a good choice vis-a-vis performance,
isthat even a valid strategy? It sounds like you are saying it is not.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company