From db76b1efbbab2441428a9ef21f7ac9ba43c52482 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 15 Feb 2016 22:15:35 +0100 Subject: [PATCH] Allow SetHintBits() to succeed if the buffer's LSN is new enough. Previously we only allowed SetHintBits() to succeed if the commit LSN of the last transaction touching the page has already been flushed to disk. We can't generally change the LSN of the page, because we don't necessarily have the required locks on the page. But the required LSN interlock does not mean the commit record has to be flushed immediately, it just requires that the commit record will be flushed before the page is written out. Therefore if the buffer LSN is newer than the commit LSN, the hint bit can be safely set. In a number of scenarios (e.g. pgbench) this noticeably increases the number of hint bits are set. But more importantly it also keeps the success rate up when flushing WAL less frequently. That was the original reason for commit 4de82f7d7, which has negative performance consequences in a number of scenarios. This will allow a followup commit to reduce the flush rate. Discussion: 20160118163908.GW10941@awork2.anarazel.de --- src/backend/utils/time/tqual.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c index 465933d5e5..0e815a9ae9 100644 --- a/src/backend/utils/time/tqual.c +++ b/src/backend/utils/time/tqual.c @@ -89,12 +89,13 @@ static bool XidInMVCCSnapshot(TransactionId xid, Snapshot snapshot); * Set commit/abort hint bits on a tuple, if appropriate at this time. * * It is only safe to set a transaction-committed hint bit if we know the - * transaction's commit record has been flushed to disk, or if the table is - * temporary or unlogged and will be obliterated by a crash anyway. We - * cannot change the LSN of the page here because we may hold only a share - * lock on the buffer, so we can't use the LSN to interlock this; we have to - * just refrain from setting the hint bit until some future re-examination - * of the tuple. + * transaction's commit record is guaranteed to be flushed to disk before the + * buffer, or if the table is temporary or unlogged and will be obliterated by + * a crash anyway. We cannot change the LSN of the page here, because we may + * hold only a share lock on the buffer, so we can only use the LSN to + * interlock this if the buffer's LSN already is newer than the commit LSN; + * otherwise we have to just refrain from setting the hint bit until some + * future re-examination of the tuple. * * We can always set hint bits when marking a transaction aborted. (Some * code in heapam.c relies on that!) @@ -122,8 +123,12 @@ SetHintBits(HeapTupleHeader tuple, Buffer buffer, /* NB: xid must be known committed here! */ XLogRecPtr commitLSN = TransactionIdGetCommitLSN(xid); - if (XLogNeedsFlush(commitLSN) && BufferIsPermanent(buffer)) - return; /* not flushed yet, so don't set hint */ + if (BufferIsPermanent(buffer) && XLogNeedsFlush(commitLSN) && + BufferGetLSNAtomic(buffer) < commitLSN) + { + /* not flushed and no LSN interlock, so don't set hint */ + return; + } } tuple->t_infomask |= infomask; -- 2.40.0