From 12bbf5dbd372a431d106e7e8c3ed50ffa5707a19 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Mon, 26 Oct 2020 01:50:08 +0300 Subject: [PATCH 1/3] Use shared lock in GetMultiXactIdMembers for offsets and members Previously the read of multixact required exclusive control locks for both offsets and members SLRUs. This could lead to the excessive lock contention. This commit we makes multixacts SLRU take advantage of SimpleLruReadPage_ReadOnly similar to clog, commit_ts and subtrans. In order to evade extra reacquiring of CLRU lock, we teach SimpleLruReadPage_ReadOnly() to take into account the current lock mode and report resulting lock mode back. Discussion: https://postgr.es/m/a7f1c4e1-1015-92a4-2bd4-6736bd13d03e%40postgrespro.ru#c496c4e75fc0605094a0e1f763e6a6ec Author: Andrey Borodin Reviewed-by: Kyotaro Horiguchi, Daniel Gustafsson Reviewed-by: Anastasia Lubennikova, Alexander Korotkov --- src/backend/access/transam/clog.c | 4 +++- src/backend/access/transam/commit_ts.c | 4 +++- src/backend/access/transam/multixact.c | 22 +++++++++++++++------- src/backend/access/transam/slru.c | 23 +++++++++++++++++------ src/backend/access/transam/subtrans.c | 4 +++- src/backend/commands/async.c | 4 +++- src/backend/storage/lmgr/predicate.c | 4 +++- src/include/access/slru.h | 2 +- src/include/storage/lwlock.h | 2 ++ 9 files changed, 50 insertions(+), 19 deletions(-) diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c index 034349aa7b9..4c372065dee 100644 --- a/src/backend/access/transam/clog.c +++ b/src/backend/access/transam/clog.c @@ -640,10 +640,11 @@ TransactionIdGetStatus(TransactionId xid, XLogRecPtr *lsn) int lsnindex; char *byteptr; XidStatus status; + LWLockMode lockmode = LW_NONE; /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(XactCtl, pageno, xid); + slotno = SimpleLruReadPage_ReadOnly(XactCtl, pageno, xid, &lockmode); byteptr = XactCtl->shared->page_buffer[slotno] + byteno; status = (*byteptr >> bshift) & CLOG_XACT_BITMASK; @@ -651,6 +652,7 @@ TransactionIdGetStatus(TransactionId xid, XLogRecPtr *lsn) lsnindex = GetLSNIndex(slotno, xid); *lsn = XactCtl->shared->group_lsn[lsnindex]; + Assert(lockmode != LW_NONE); LWLockRelease(XactSLRULock); return status; diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c index cb8a9688018..c19b0b5399e 100644 --- a/src/backend/access/transam/commit_ts.c +++ b/src/backend/access/transam/commit_ts.c @@ -288,6 +288,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts, CommitTimestampEntry entry; TransactionId oldestCommitTsXid; TransactionId newestCommitTsXid; + LWLockMode lockmode = LW_NONE; if (!TransactionIdIsValid(xid)) ereport(ERROR, @@ -342,7 +343,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts, } /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, xid); + slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, xid, &lockmode); memcpy(&entry, CommitTsCtl->shared->page_buffer[slotno] + SizeOfCommitTimestampEntry * entryno, @@ -352,6 +353,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts, if (nodeid) *nodeid = entry.nodeid; + Assert(lockmode != LW_NONE); LWLockRelease(CommitTsSLRULock); return *ts != 0; } diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index 43653fe5721..ccbce90f0ea 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -1237,6 +1237,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members, MultiXactId tmpMXact; MultiXactOffset nextOffset; MultiXactMember *ptr; + LWLockMode lockmode = LW_NONE; debug_elog3(DEBUG2, "GetMembers: asked for %u", multi); @@ -1340,12 +1341,13 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members, * time on every multixact creation. */ retry: - LWLockAcquire(MultiXactOffsetSLRULock, LW_EXCLUSIVE); pageno = MultiXactIdToOffsetPage(multi); entryno = MultiXactIdToOffsetEntry(multi); - slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi); + /* lock is acquired by SimpleLruReadPage_ReadOnly */ + slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, + multi, &lockmode); offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno]; offptr += entryno; offset = *offptr; @@ -1377,7 +1379,8 @@ retry: entryno = MultiXactIdToOffsetEntry(tmpMXact); if (pageno != prev_pageno) - slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, tmpMXact); + slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, + tmpMXact, &lockmode); offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno]; offptr += entryno; @@ -1395,14 +1398,14 @@ retry: length = nextMXOffset - offset; } + Assert(lockmode != LW_NONE); LWLockRelease(MultiXactOffsetSLRULock); ptr = (MultiXactMember *) palloc(length * sizeof(MultiXactMember)); *members = ptr; /* Now get the members themselves. */ - LWLockAcquire(MultiXactMemberSLRULock, LW_EXCLUSIVE); - + lockmode = LW_NONE; truelength = 0; prev_pageno = -1; for (i = 0; i < length; i++, offset++) @@ -1418,7 +1421,8 @@ retry: if (pageno != prev_pageno) { - slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi); + slotno = SimpleLruReadPage_ReadOnly(MultiXactMemberCtl, pageno, + multi, &lockmode); prev_pageno = pageno; } @@ -1441,6 +1445,7 @@ retry: truelength++; } + Assert(lockmode != LW_NONE); LWLockRelease(MultiXactMemberSLRULock); /* @@ -2733,6 +2738,7 @@ find_multixact_start(MultiXactId multi, MultiXactOffset *result) int entryno; int slotno; MultiXactOffset *offptr; + LWLockMode lockmode = LW_NONE; Assert(MultiXactState->finishedStartup); @@ -2749,10 +2755,12 @@ find_multixact_start(MultiXactId multi, MultiXactOffset *result) return false; /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, multi); + slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, + multi, &lockmode); offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno]; offptr += entryno; offset = *offptr; + Assert(lockmode != LW_NONE); LWLockRelease(MultiXactOffsetSLRULock); *result = offset; diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c index 16a78986971..ca0a23ab875 100644 --- a/src/backend/access/transam/slru.c +++ b/src/backend/access/transam/slru.c @@ -487,17 +487,23 @@ SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok, * Return value is the shared-buffer slot number now holding the page. * The buffer's LRU access info is updated. * - * Control lock must NOT be held at entry, but will be held at exit. - * It is unspecified whether the lock will be shared or exclusive. + * Control lock must be held in *lock_mode mode, which may be LW_NONE. Control + * lock will be held at exit in at least shared mode. Resulting control lock + * mode is set to *lock_mode. */ int -SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid) +SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid, + LWLockMode *lock_mode) { SlruShared shared = ctl->shared; int slotno; /* Try to find the page while holding only shared lock */ - LWLockAcquire(shared->ControlLock, LW_SHARED); + if (*lock_mode == LW_NONE) + { + LWLockAcquire(shared->ControlLock, LW_SHARED); + *lock_mode = LW_SHARED; + } /* See if page is already in a buffer */ for (slotno = 0; slotno < shared->num_slots; slotno++) @@ -517,8 +523,13 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid) } /* No luck, so switch to normal exclusive lock and do regular read */ - LWLockRelease(shared->ControlLock); - LWLockAcquire(shared->ControlLock, LW_EXCLUSIVE); + if (*lock_mode != LW_EXCLUSIVE) + { + Assert(*lock_mode == LW_NONE); + LWLockRelease(shared->ControlLock); + LWLockAcquire(shared->ControlLock, LW_EXCLUSIVE); + *lock_mode = LW_EXCLUSIVE; + } return SimpleLruReadPage(ctl, pageno, true, xid); } diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c index 0111e867c79..7bb15431893 100644 --- a/src/backend/access/transam/subtrans.c +++ b/src/backend/access/transam/subtrans.c @@ -113,6 +113,7 @@ SubTransGetParent(TransactionId xid) int slotno; TransactionId *ptr; TransactionId parent; + LWLockMode lockmode = LW_NONE; /* Can't ask about stuff that might not be around anymore */ Assert(TransactionIdFollowsOrEquals(xid, TransactionXmin)); @@ -123,12 +124,13 @@ SubTransGetParent(TransactionId xid) /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, xid); + slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, xid, &lockmode); ptr = (TransactionId *) SubTransCtl->shared->page_buffer[slotno]; ptr += entryno; parent = *ptr; + Assert(lockmode != LW_NONE); LWLockRelease(SubtransSLRULock); return parent; diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 8dbcace3f93..8b419575590 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -2002,6 +2002,7 @@ asyncQueueReadAllNotifications(void) int curoffset = QUEUE_POS_OFFSET(pos); int slotno; int copysize; + LWLockMode lockmode = LW_NONE; /* * We copy the data from SLRU into a local buffer, so as to avoid @@ -2010,7 +2011,7 @@ asyncQueueReadAllNotifications(void) * part of the page we will actually inspect. */ slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, curpage, - InvalidTransactionId); + InvalidTransactionId, &lockmode); if (curpage == QUEUE_POS_PAGE(head)) { /* we only want to read as far as head */ @@ -2027,6 +2028,7 @@ asyncQueueReadAllNotifications(void) NotifyCtl->shared->page_buffer[slotno] + curoffset, copysize); /* Release lock that we got from SimpleLruReadPage_ReadOnly() */ + Assert(lockmode != LW_NONE); LWLockRelease(NotifySLRULock); /* diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 8a365b400c6..a4df90a8aeb 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -924,6 +924,7 @@ SerialGetMinConflictCommitSeqNo(TransactionId xid) TransactionId tailXid; SerCommitSeqNo val; int slotno; + LWLockMode lockmode = LW_NONE; Assert(TransactionIdIsValid(xid)); @@ -946,8 +947,9 @@ SerialGetMinConflictCommitSeqNo(TransactionId xid) * but will return with that lock held, which must then be released. */ slotno = SimpleLruReadPage_ReadOnly(SerialSlruCtl, - SerialPage(xid), xid); + SerialPage(xid), xid, &lockmode); val = SerialValue(slotno, xid); + Assert(lockmode != LW_NONE); LWLockRelease(SerialSLRULock); return val; } diff --git a/src/include/access/slru.h b/src/include/access/slru.h index b39b43504d8..4b66d3b592b 100644 --- a/src/include/access/slru.h +++ b/src/include/access/slru.h @@ -142,7 +142,7 @@ extern int SimpleLruZeroPage(SlruCtl ctl, int pageno); extern int SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok, TransactionId xid); extern int SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, - TransactionId xid); + TransactionId xid, LWLockMode *lock_mode); extern void SimpleLruWritePage(SlruCtl ctl, int slotno); extern void SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied); extern void SimpleLruTruncate(SlruCtl ctl, int cutoffPage); diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index af9b41795d2..2684c8e51c9 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -129,6 +129,8 @@ extern PGDLLIMPORT int NamedLWLockTrancheRequests; typedef enum LWLockMode { + LW_NONE, /* Not a lock mode. Indicates that there is + * no lock. */ LW_EXCLUSIVE, LW_SHARED, LW_WAIT_UNTIL_FREE /* A special mode used in PGPROC->lwWaitMode, -- 2.14.3