public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andrey M. Borodin <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: MultiXact\SLRU buffers configuration
Date: Wed, 8 Jul 2020 12:03:54 +0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
> 2 июля 2020 г., в 17:02, Daniel Gustafsson <[email protected]> написал(а):
>
>> On 15 May 2020, at 02:03, Kyotaro Horiguchi <[email protected]> wrote:
>
>> Generally in such cases, condition variables would work. In the
>> attached PoC, the reader side gets no penalty in the "likely" code
>> path. The writer side always calls ConditionVariableBroadcast but the
>> waiter list is empty in almost all cases. But I couldn't cause the
>> situation where the sleep 1000u is reached.
>
> The submitted patch no longer applies, can you please submit an updated
> version? I'm marking the patch Waiting on Author in the meantime.
Thanks, Daniel! PFA V2
Best regards, Andrey Borodin.
Attachments:
[application/octet-stream] v2-0001-Use-shared-lock-in-GetMultiXactIdMembers-for-offs.patch (7.6K, ../[email protected]/2-v2-0001-Use-shared-lock-in-GetMultiXactIdMembers-for-offs.patch)
download | inline diff:
From 264b475fbf6ee519dc3b4eb2cca25145d2aaa569 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Sat, 9 May 2020 21:19:18 +0500
Subject: [PATCH v2 1/4] 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 f3da40ae01..3614d0c774 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(XactCtl, pageno, xid);
+ slotno = SimpleLruReadPage_ReadOnly(XactCtl, pageno, xid, false);
byteptr = XactCtl->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 9cdb136435..5fbbf446e3 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 79ec21c75a..241d9dd78d 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(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, 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(MultiXactMemberSLRULock, LW_EXCLUSIVE);
+ LWLockAcquire(MultiXactMemberSLRULock, 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 61249f4a12..96d0def5c2 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -475,17 +475,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 f33ae407a6..8d8e0d9ec7 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 71b7577afc..70085fc037 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -2012,7 +2012,7 @@ asyncQueueReadAllNotifications(void)
* part of the page we will actually inspect.
*/
slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, 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 d24919f76b..e2a7d00d37 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -948,7 +948,7 @@ 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, false);
val = SerialValue(slotno, xid);
LWLockRelease(SerialSLRULock);
return val;
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 61fbc80ef0..471c692021 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -140,7 +140,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)
[application/octet-stream] v2-0002-Make-MultiXact-local-cache-size-configurable.patch (3.7K, ../[email protected]/3-v2-0002-Make-MultiXact-local-cache-size-configurable.patch)
download | inline diff:
From 520686d6cc22a599bc06b91391171ca218c3db75 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Mon, 11 May 2020 16:17:02 +0500
Subject: [PATCH v2 2/4] Make MultiXact local cache size configurable
---
doc/src/sgml/config.sgml | 16 ++++++++++++++++
src/backend/access/transam/multixact.c | 2 +-
src/backend/utils/init/globals.c | 1 +
src/backend/utils/misc/guc.c | 10 ++++++++++
src/include/miscadmin.h | 1 +
5 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 94d99373a7..31d7c1f4b3 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1777,6 +1777,22 @@ include_dir 'conf.d'
</listitem>
</varlistentry>
+ <varlistentry id="guc-multixact-local-cache-entries" xreflabel="multixact_local-cache-entries">
+ <term><varname>multixact_local_cache_entries</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>multixact_local_cache_entries</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specifies the number cached MultiXact by backend. Any SLRU lookup is preceeded
+ by cache lookup. Higher numbers of cache size help to deduplicate lock sets, while
+ lower values make cache lookup faster.
+ It defaults to 256.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-max-stack-depth" xreflabel="max_stack_depth">
<term><varname>max_stack_depth</varname> (<type>integer</type>)
<indexterm>
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 241d9dd78d..445b6258a4 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1589,7 +1589,7 @@ mXactCachePut(MultiXactId multi, int nmembers, MultiXactMember *members)
qsort(entry->members, nmembers, sizeof(MultiXactMember), mxactMemberComparator);
dlist_push_head(&MXactCache, &entry->node);
- if (MXactCacheMembers++ >= MAX_CACHE_ENTRIES)
+ if (MXactCacheMembers++ >= multixact_local_cache_entries)
{
dlist_node *node;
mXactCacheEnt *entry;
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 01e739da40..f404167ad0 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -151,3 +151,4 @@ double vacuum_cleanup_index_scale_factor;
int multixact_offsets_slru_buffers = 8;
int multixact_members_slru_buffers = 16;
+int multixact_local_cache_entries = 256;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index d1fbb75cd5..6443f595cc 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2273,6 +2273,16 @@ static struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"multixact_local_cache_entries", PGC_SUSET, RESOURCES_MEM,
+ gettext_noop("Sets the number cached MultiXact by backend."),
+ NULL
+ },
+ &multixact_local_cache_entries,
+ 256, 2, INT_MAX / 2,
+ NULL, NULL, NULL
+ },
+
{
{"temp_buffers", PGC_USERSET, RESOURCES_MEM,
gettext_noop("Sets the maximum number of temporary buffers used by each session."),
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index bb1475ad2b..9ce2adea63 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -163,6 +163,7 @@ extern PGDLLIMPORT int max_worker_processes;
extern PGDLLIMPORT int max_parallel_workers;
extern PGDLLIMPORT int multixact_offsets_slru_buffers;
extern PGDLLIMPORT int multixact_members_slru_buffers;
+extern PGDLLIMPORT int multixact_local_cache_entries;
extern PGDLLIMPORT int MyProcPid;
extern PGDLLIMPORT pg_time_t MyStartTime;
--
2.24.2 (Apple Git-127)
[application/octet-stream] v2-0003-Add-conditional-variable-to-wait-for-next-MultXac.patch (3.4K, ../[email protected]/4-v2-0003-Add-conditional-variable-to-wait-for-next-MultXac.patch)
download | inline diff:
From ddde253b10b87d060d34f8d7250be27a27c18c0a Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Fri, 22 May 2020 14:13:33 +0500
Subject: [PATCH v2 3/4] Add conditional variable to wait for next MultXact
offset in edge case
---
src/backend/access/transam/multixact.c | 23 ++++++++++++++++++++++-
src/backend/postmaster/pgstat.c | 2 ++
src/include/pgstat.h | 1 +
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 445b6258a4..02de9a7c13 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -82,6 +82,7 @@
#include "lib/ilist.h"
#include "miscadmin.h"
#include "pg_trace.h"
+#include "pgstat.h"
#include "postmaster/autovacuum.h"
#include "storage/lmgr.h"
#include "storage/pmsignal.h"
@@ -233,6 +234,7 @@ typedef struct MultiXactStateData
/* support for members anti-wraparound measures */
MultiXactOffset offsetStopLimit; /* known if oldestOffsetKnown */
+ ConditionVariable nextoff_cv;
/*
* Per-backend data starts here. We have two arrays stored in the area
* immediately following the MultiXactStateData struct. Each is indexed by
@@ -873,6 +875,14 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
/* Exchange our lock */
LWLockRelease(MultiXactOffsetSLRULock);
+ /*
+ * Let everybody know the offset of this mxid is recorded now. The waiters
+ * are waiting for the offset of the mxid next of the target to know the
+ * number of members of the target mxid, so we don't need to wait for
+ * members of this mxid are recorded.
+ */
+ ConditionVariableBroadcast(&MultiXactState->nextoff_cv);
+
LWLockAcquire(MultiXactMemberSLRULock, LW_EXCLUSIVE);
prev_pageno = -1;
@@ -1367,9 +1377,19 @@ retry:
if (nextMXOffset == 0)
{
/* Corner case 2: next multixact is still being filled in */
+
+ /*
+ * The recorder of the next mxid is just before writing the offset.
+ * Wait for the offset to be written.
+ */
+ ConditionVariablePrepareToSleep(&MultiXactState->nextoff_cv);
+
LWLockRelease(MultiXactOffsetSLRULock);
CHECK_FOR_INTERRUPTS();
- pg_usleep(1000L);
+
+ ConditionVariableSleep(&MultiXactState->nextoff_cv,
+ WAIT_EVENT_WAIT_NEXT_MXMEMBERS);
+ ConditionVariableCancelSleep();
goto retry;
}
@@ -1847,6 +1867,7 @@ MultiXactShmemInit(void)
/* Make sure we zero out the per-backend state */
MemSet(MultiXactState, 0, SHARED_MULTIXACT_STATE_SIZE);
+ ConditionVariableInit(&MultiXactState->nextoff_cv);
}
else
Assert(found);
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index edfa774ee4..4bce534c7a 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3877,6 +3877,8 @@ pgstat_get_wait_ipc(WaitEventIPC w)
break;
case WAIT_EVENT_XACT_GROUP_UPDATE:
event_name = "XactGroupUpdate";
+ case WAIT_EVENT_WAIT_NEXT_MXMEMBERS:
+ event_name = "MultiXact/WaitNextXactMembers";
break;
/* no default case, so that compiler will warn */
}
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 1387201382..26f828597b 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -887,6 +887,7 @@ typedef enum
WAIT_EVENT_REPLICATION_SLOT_DROP,
WAIT_EVENT_SAFE_SNAPSHOT,
WAIT_EVENT_SYNC_REP,
+ WAIT_EVENT_WAIT_NEXT_MXMEMBERS,
WAIT_EVENT_XACT_GROUP_UPDATE
} WaitEventIPC;
--
2.24.2 (Apple Git-127)
view thread (81+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: MultiXact\SLRU buffers configuration
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox