From 84330d0c1fd3dd76f95ce82dd5856a9de52b5b23 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Thu, 23 Jul 2015 01:30:15 +0300 Subject: [PATCH] Fix off-by-one error in calculating subtrans/multixact truncation point. If there were no subtransactions (or multixacts) active, we would calculate the oldestxid == next xid. That's correct, but if next XID happens to be on the next pg_subtrans (pg_multixact) page, the page does not exist yet, and SimpleLruTruncate will produce an "apparent wraparound" warning. The warning is harmless in this case, but looks very alarming to users. Backpatch to all supported versions. Patch and analysis by Thomas Munro. --- src/backend/access/transam/multixact.c | 14 ++++++++++---- src/backend/access/transam/subtrans.c | 7 ++++++- src/include/access/multixact.h | 1 + 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index 83b4d432728..51685d52e02 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -89,6 +89,8 @@ #define MXOffsetToMemberEntry(xid) \ ((xid) % (TransactionId) MULTIXACT_MEMBERS_PER_PAGE) +#define PreviousMultiXactId(xid) \ + ((xid) == FirstMultiXactId ? MaxMultiXactId : (xid) - 1) /* * Links to shared-memory data structures for MultiXact control @@ -1907,17 +1909,21 @@ TruncateMultiXact(void) } /* - * The cutoff point is the start of the segment containing oldestMXact. We - * pass the *page* containing oldestMXact to SimpleLruTruncate. + * The cutoff point is the start of the segment containing oldestMXact. + * We step back one multixact to avoid passing a cutoff page that hasn't + * been created yet in the rare case that oldestMXact would be the first + * item on a page and oldestMXact == nextMXact. In that case, if we + * didn't subtract one, we'd trigger SimpleLruTruncate's wraparound + * detection. */ - cutoffPage = MultiXactIdToOffsetPage(oldestMXact); + cutoffPage = MultiXactIdToOffsetPage(PreviousMultiXactId(oldestMXact)); SimpleLruTruncate(MultiXactOffsetCtl, cutoffPage); /* * Also truncate MultiXactMember at the previously determined offset. */ - cutoffPage = MXOffsetToMemberPage(oldestOffset); + cutoffPage = MXOffsetToMemberPage(oldestOffset - 1); SimpleLruTruncate(MultiXactMemberCtl, cutoffPage); diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c index 6665171a767..2ad7477d430 100644 --- a/src/backend/access/transam/subtrans.c +++ b/src/backend/access/transam/subtrans.c @@ -340,8 +340,13 @@ TruncateSUBTRANS(TransactionId oldestXact) /* * The cutoff point is the start of the segment containing oldestXact. We - * pass the *page* containing oldestXact to SimpleLruTruncate. + * pass the *page* containing oldestXact to SimpleLruTruncate. We step + * back one transaction to avoid passing a cutoff page that hasn't been + * created yet in the rare case that oldestXact would be the first item on + * a page and oldestXact == next XID. In that case, if we didn't subtract + * one, we'd trigger SimpleLruTruncate's wraparound detection. */ + TransactionIdRetreat(oldestXact); cutoffPage = TransactionIdToPage(oldestXact); SimpleLruTruncate(SubTransCtl, cutoffPage); diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h index 7f46c3eeea4..22419bbf100 100644 --- a/src/include/access/multixact.h +++ b/src/include/access/multixact.h @@ -15,6 +15,7 @@ #define InvalidMultiXactId ((MultiXactId) 0) #define FirstMultiXactId ((MultiXactId) 1) +#define MaxMultiXactId ((MultiXactId) 0xFFFFFFFF) #define MultiXactIdIsValid(multi) ((multi) != InvalidMultiXactId)