From 0a0ebc6391c33ca77260656d886224c71fb8ad96 Mon Sep 17 00:00:00 2001 From: Andrey Borodin Date: Sat, 9 May 2020 21:19:18 +0500 Subject: [PATCH v1 2/3] Use shared lock in GetMultiXactIdMembers for offsets and members Previously read of multixact required exclusive control locks in offstes and members SLRUs. This could lead to contention. In this commit we take advantge of SimpleLruReadPage_ReadOnly similar to CLOG usage. --- src/backend/access/transam/clog.c | 2 +- src/backend/access/transam/commit_ts.c | 2 +- src/backend/access/transam/multixact.c | 12 ++++++------ src/backend/access/transam/slru.c | 8 +++++--- src/backend/access/transam/subtrans.c | 2 +- src/backend/commands/async.c | 2 +- src/backend/storage/lmgr/predicate.c | 2 +- src/include/access/slru.h | 2 +- 8 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c index f8e7670f8d..2ae024608e 100644 --- a/src/backend/access/transam/clog.c +++ b/src/backend/access/transam/clog.c @@ -643,7 +643,7 @@ TransactionIdGetStatus(TransactionId xid, XLogRecPtr *lsn) /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(ClogCtl, pageno, xid); + slotno = SimpleLruReadPage_ReadOnly(ClogCtl, pageno, xid, false); byteptr = ClogCtl->shared->page_buffer[slotno] + byteno; status = (*byteptr >> bshift) & CLOG_XACT_BITMASK; diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c index 630df672cc..1efa269339 100644 --- a/src/backend/access/transam/commit_ts.c +++ b/src/backend/access/transam/commit_ts.c @@ -342,7 +342,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts, } /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, xid); + slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, xid, false); memcpy(&entry, CommitTsCtl->shared->page_buffer[slotno] + SizeOfCommitTimestampEntry * entryno, diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index d2ac029b98..5387a3602d 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -1321,12 +1321,12 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members, * time on every multixact creation. */ retry: - LWLockAcquire(MultiXactOffsetControlLock, 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, false); offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno]; offptr += entryno; offset = *offptr; @@ -1358,7 +1358,7 @@ retry: entryno = MultiXactIdToOffsetEntry(tmpMXact); if (pageno != prev_pageno) - slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, tmpMXact); + slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, tmpMXact, true); offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno]; offptr += entryno; @@ -1382,7 +1382,7 @@ retry: *members = ptr; /* Now get the members themselves. */ - LWLockAcquire(MultiXactMemberControlLock, LW_EXCLUSIVE); + LWLockAcquire(MultiXactMemberControlLock, LW_SHARED); truelength = 0; prev_pageno = -1; @@ -1399,7 +1399,7 @@ retry: if (pageno != prev_pageno) { - slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi); + slotno = SimpleLruReadPage_ReadOnly(MultiXactMemberCtl, pageno, multi, true); prev_pageno = pageno; } @@ -2745,7 +2745,7 @@ 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, false); offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno]; offptr += entryno; offset = *offptr; diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c index b2316af779..ae63ad4e6c 100644 --- a/src/backend/access/transam/slru.c +++ b/src/backend/access/transam/slru.c @@ -470,17 +470,19 @@ 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. + * Control lock will be held at exit. If lock_held is true at least + * shared control lock must be held. * It is unspecified whether the lock will be shared or exclusive. */ int -SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid) +SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid, bool lock_held) { SlruShared shared = ctl->shared; int slotno; /* Try to find the page while holding only shared lock */ - LWLockAcquire(shared->ControlLock, LW_SHARED); + if (!lock_held) + LWLockAcquire(shared->ControlLock, LW_SHARED); /* See if page is already in a buffer */ for (slotno = 0; slotno < shared->num_slots; slotno++) diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c index 25d7d739cf..98700f26a7 100644 --- a/src/backend/access/transam/subtrans.c +++ b/src/backend/access/transam/subtrans.c @@ -123,7 +123,7 @@ SubTransGetParent(TransactionId xid) /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, xid); + slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, xid, false); ptr = (TransactionId *) SubTransCtl->shared->page_buffer[slotno]; ptr += entryno; diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 0c9d20ebfc..a12447ecb5 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -2005,7 +2005,7 @@ asyncQueueReadAllNotifications(void) * of the page we will actually inspect. */ slotno = SimpleLruReadPage_ReadOnly(AsyncCtl, curpage, - InvalidTransactionId); + InvalidTransactionId, false); if (curpage == QUEUE_POS_PAGE(head)) { /* we only want to read as far as head */ diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 654584b77a..2969992a90 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -948,7 +948,7 @@ OldSerXidGetMinConflictCommitSeqNo(TransactionId xid) * but will return with that lock held, which must then be released. */ slotno = SimpleLruReadPage_ReadOnly(OldSerXidSlruCtl, - OldSerXidPage(xid), xid); + OldSerXidPage(xid), xid, false); val = OldSerXidValue(slotno, xid); LWLockRelease(OldSerXidLock); return val; diff --git a/src/include/access/slru.h b/src/include/access/slru.h index 00dbd803e1..916dd8203e 100644 --- a/src/include/access/slru.h +++ b/src/include/access/slru.h @@ -144,7 +144,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, bool lock_held); extern void SimpleLruWritePage(SlruCtl ctl, int slotno); extern void SimpleLruFlush(SlruCtl ctl, bool allow_redirtied); extern void SimpleLruTruncate(SlruCtl ctl, int cutoffPage); -- 2.24.2 (Apple Git-127)