From b4529005fd387e863bfa9eb863629b1183c0449c Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Sat, 28 May 2022 15:02:08 +0900 Subject: [PATCH] Revert "Add single-item cache when looking at topmost XID of a subtrans XID" This reverts commit 06f5295 as per issues with this approach, both in terms of efficiency impact and stability. First, contrary to the single-item cache for transaction IDs in transam.c, the cache may finish by not be hit for a long time, and without an invalidation mechanism to clear it, it would cause inconsistent results on wraparound for example. Second, the use of SubTransGetTopmostTransaction() for the caching has a limited impact on performance. SubTransGetParent() could have more impact, though the benchmarking of the single-item approach still needs to be proved, particularly under the conditions where SLRU lookups are stressed in parallel with overflowed snapshots (aka more than 64 subxids generated, for example). After discussion with Andres Freund. Discussion: https://postgr.es/m/20220524235250.gtt3uu5zktfkr4hv@alap3.anarazel.de --- src/backend/access/transam/subtrans.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c index 7240454ca4..66d3548155 100644 --- a/src/backend/access/transam/subtrans.c +++ b/src/backend/access/transam/subtrans.c @@ -54,14 +54,6 @@ #define TransactionIdToPage(xid) ((xid) / (TransactionId) SUBTRANS_XACTS_PER_PAGE) #define TransactionIdToEntry(xid) ((xid) % (TransactionId) SUBTRANS_XACTS_PER_PAGE) -/* - * Single-item cache for results of SubTransGetTopmostTransaction(). It's - * worth having such a cache because we frequently find ourselves repeatedly - * checking the same XID, for example when scanning a table just after a - * bulk insert, update, or delete. - */ -static TransactionId cachedFetchSubXid = InvalidTransactionId; -static TransactionId cachedFetchTopmostXid = InvalidTransactionId; /* * Link to shared-memory data structures for SUBTRANS control @@ -163,13 +155,6 @@ SubTransGetTopmostTransaction(TransactionId xid) /* Can't ask about stuff that might not be around anymore */ Assert(TransactionIdFollowsOrEquals(xid, TransactionXmin)); - /* - * Before going to the subtrans log, check our single item cache to see if - * we know the result from a previous/recent request. - */ - if (TransactionIdEquals(xid, cachedFetchSubXid)) - return cachedFetchTopmostXid; - while (TransactionIdIsValid(parentXid)) { previousXid = parentXid; @@ -189,9 +174,6 @@ SubTransGetTopmostTransaction(TransactionId xid) Assert(TransactionIdIsValid(previousXid)); - cachedFetchSubXid = xid; - cachedFetchTopmostXid = previousXid; - return previousXid; }