public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andrey Borodin <[email protected]>
To: Gilles Darold <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Alexander Korotkov <[email protected]>
Cc: Anastasia Lubennikova <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: MultiXact\SLRU buffers configuration
Date: Tue, 8 Dec 2020 22:52:52 +0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<CAPpHfduekT=BrM3rLqvo4ajzKYmDk+6aCoYJrNHhK0Je+v2z=Q@mail.gmail.com>
	<[email protected]>
	<CAPpHfduT0Kb+8fMt4t0EF5G_3hig+wYgeRadQnNq20F4VxP7LA@mail.gmail.com>
	<CAPpHfdsUtSGxRrKn65F_wrE66sNztNY4TpDGDHh2kvAZZDOMqA@mail.gmail.com>
	<20201028013651.de5cj2xadgmba5nf@development>
	<[email protected]>
	<20201028233243.ygm6yqlynkqpzekr@development>
	<[email protected]>
	<20201029134933.xd4mh2cofuf6tdfz@development>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Hi Gilles!

Many thanks for your message!

> 8 дек. 2020 г., в 21:05, Gilles Darold <[email protected]> написал(а):
> 
> I know that this report is not really helpful 

Quite contrary - this benchmarks prove that controllable reproduction exists. I've rebased patches for PG11. Can you please benchmark them (without extending SLRU)?

Best regards, Andrey Borodin.



Attachments:

  [application/octet-stream] v1106-0001-Use-shared-lock-in-GetMultiXactIdMembers-for-o.patch (12.7K, ../[email protected]/2-v1106-0001-Use-shared-lock-in-GetMultiXactIdMembers-for-o.patch)
  download | inline diff:
From dee5bf87314606e020a574b85cf1e9cbe066588d Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Tue, 27 Oct 2020 19:29:56 +0300
Subject: [PATCH v1106 1/4] 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      |  3 ++-
 src/backend/access/transam/commit_ts.c |  3 ++-
 src/backend/access/transam/multixact.c | 21 ++++++++++++++-------
 src/backend/access/transam/slru.c      | 23 +++++++++++++++++------
 src/backend/access/transam/subtrans.c  |  3 ++-
 src/backend/commands/async.c           |  6 ++++--
 src/backend/storage/lmgr/lwlock.c      |  3 +++
 src/backend/storage/lmgr/predicate.c   |  4 +++-
 src/include/access/slru.h              |  8 ++++----
 src/include/storage/lwlock.h           |  2 ++
 10 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index a8d1080f17..bdfbd10f96 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(ClogCtl, pageno, xid);
+	slotno = SimpleLruReadPage_ReadOnly(ClogCtl, pageno, xid, &lockmode);
 	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 6c4911d9bc..d7b6f583e1 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,
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 365daf153a..5ca10a139a 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1216,6 +1216,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
 	MultiXactId tmpMXact;
 	MultiXactOffset nextOffset;
 	MultiXactMember *ptr;
+	LWLockMode	lockmode = LW_NONE;
 
 	debug_elog3(DEBUG2, "GetMembers: asked for %u", multi);
 
@@ -1319,12 +1320,13 @@ 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, &lockmode);
 	offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
 	offptr += entryno;
 	offset = *offptr;
@@ -1356,7 +1358,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;
@@ -1380,8 +1383,7 @@ retry:
 	*members = ptr;
 
 	/* Now get the members themselves. */
-	LWLockAcquire(MultiXactMemberControlLock, LW_EXCLUSIVE);
-
+	lockmode = LW_NONE;
 	truelength = 0;
 	prev_pageno = -1;
 	for (i = 0; i < length; i++, offset++)
@@ -1397,7 +1399,8 @@ retry:
 
 		if (pageno != prev_pageno)
 		{
-			slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi);
+			slotno = SimpleLruReadPage_ReadOnly(MultiXactMemberCtl, pageno,
+												multi, &lockmode);
 			prev_pageno = pageno;
 		}
 
@@ -1420,6 +1423,7 @@ retry:
 		truelength++;
 	}
 
+	Assert(lockmode != LW_NONE);
 	LWLockRelease(MultiXactMemberControlLock);
 
 	/*
@@ -2723,6 +2727,7 @@ find_multixact_start(MultiXactId multi, MultiXactOffset *result)
 	int			entryno;
 	int			slotno;
 	MultiXactOffset *offptr;
+	LWLockMode	lockmode = LW_NONE;
 
 	Assert(MultiXactState->finishedStartup);
 
@@ -2743,10 +2748,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(MultiXactOffsetControlLock);
 
 	*result = offset;
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index cfe9513827..f86950eb51 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -460,17 +460,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++)
@@ -486,8 +492,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 ef63b6d98a..440a21415a 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,7 +124,7 @@ 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;
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 7f4e11bd90..4ebc069a5d 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -1834,6 +1834,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
@@ -1842,7 +1843,7 @@ asyncQueueReadAllNotifications(void)
 			 * of the page we will actually inspect.
 			 */
 			slotno = SimpleLruReadPage_ReadOnly(AsyncCtl, curpage,
-												InvalidTransactionId);
+												InvalidTransactionId, &lockmode);
 			if (curpage == QUEUE_POS_PAGE(head))
 			{
 				/* we only want to read as far as head */
@@ -1859,7 +1860,8 @@ asyncQueueReadAllNotifications(void)
 				   AsyncCtl->shared->page_buffer[slotno] + curoffset,
 				   copysize);
 			/* Release lock that we got from SimpleLruReadPage_ReadOnly() */
-			LWLockRelease(AsyncCtlLock);
+			Assert(lockmode != LW_NONE);
+			LWLockRelease(AsyncCtl);
 
 			/*
 			 * Process messages up to the stop position, end of page, or an
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 5aae233995..066dbb8ccf 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -982,6 +982,8 @@ LWLockWakeup(LWLock *lock)
 static void
 LWLockQueueSelf(LWLock *lock, LWLockMode mode)
 {
+	Assert(mode != LW_NONE);
+
 	/*
 	 * If we don't have a PGPROC structure, there's no way to wait. This
 	 * should never occur, since MyProc should only be null during shared
@@ -1742,6 +1744,7 @@ LWLockRelease(LWLock *lock)
 		elog(ERROR, "lock %s is not held", T_NAME(lock));
 
 	mode = held_lwlocks[i].mode;
+	Assert(mode == LW_EXCLUSIVE || mode == LW_SHARED);
 
 	num_held_lwlocks--;
 	for (; i < num_held_lwlocks; i++)
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 002d040ba6..9513fd915b 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -908,6 +908,7 @@ OldSerXidGetMinConflictCommitSeqNo(TransactionId xid)
 	TransactionId tailXid;
 	SerCommitSeqNo val;
 	int			slotno;
+	LWLockMode	lockmode = LW_NONE;
 
 	Assert(TransactionIdIsValid(xid));
 
@@ -930,8 +931,9 @@ 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, &lockmode);
 	val = OldSerXidValue(slotno, xid);
+	Assert(lockmode != LW_NONE);
 	LWLockRelease(OldSerXidLock);
 	return val;
 }
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 0e89e48c97..51a3df62e0 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -141,10 +141,10 @@ extern Size SimpleLruShmemSize(int nslots, int nlsns);
 extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 			  LWLock *ctllock, const char *subdir, int tranche_id);
 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);
+extern int	SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok,
+							  TransactionId xid);
+extern int	SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno,
+									   TransactionId xid, LWLockMode *lock_mode);
 extern void SimpleLruWritePage(SlruCtl ctl, int slotno);
 extern void SimpleLruFlush(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 c21bfe2f66..3263c0fe62 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -131,6 +131,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->lwlockMode,
-- 
2.24.3 (Apple Git-128)



  [application/octet-stream] v1106-0002-Make-MultiXact-local-cache-size-configurable.patch (4.8K, ../[email protected]/3-v1106-0002-Make-MultiXact-local-cache-size-configurable.patch)
  download | inline diff:
From 649c5c25349c32aecf02fad46197234af95f9860 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 26 Oct 2020 03:44:50 +0300
Subject: [PATCH v1106 2/4] Make MultiXact local cache size configurable

---
 doc/src/sgml/config.sgml               | 37 ++++++++++++++++++++++++++
 src/backend/access/transam/multixact.c |  2 +-
 src/backend/utils/init/globals.c       |  2 ++
 src/backend/utils/misc/guc.c           | 10 +++++++
 src/include/miscadmin.h                |  2 ++
 5 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3570b422be..15fb2f2bde 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1637,6 +1637,43 @@ include_dir 'conf.d'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-logical-decoding-work-mem" xreflabel="logical_decoding_work_mem">
+      <term><varname>logical_decoding_work_mem</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>logical_decoding_work_mem</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the maximum amount of memory to be used by logical decoding,
+        before some of the decoded changes are written to local disk. This
+        limits the amount of memory used by logical streaming replication
+        connections. It defaults to 64 megabytes (<literal>64MB</literal>).
+        Since each replication connection only uses a single buffer of this size,
+        and an installation normally doesn't have many such connections
+        concurrently (as limited by <varname>max_wal_senders</varname>), it's
+        safe to set this value significantly higher than <varname>work_mem</varname>,
+        reducing the amount of decoded changes written to disk.
+       </para>
+      </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 5ca10a139a..6203be0aa3 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1591,7 +1591,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 f7d6617a13..22af834150 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -147,3 +147,5 @@ int			VacuumCostBalance = 0;	/* working state for vacuum */
 bool		VacuumCostActive = false;
 
 double		vacuum_cleanup_index_scale_factor;
+
+int         multixact_local_cache_entries = 256;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 710344c72b..b54a063782 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2026,6 +2026,16 @@ static struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"multixact_local_cache_entries", PGC_SUSET, RESOURCES_MEM,
+			gettext_noop("Sets the number of 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 8024145535..11c3c8e6c5 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -161,6 +161,8 @@ extern PGDLLIMPORT int MaxConnections;
 extern PGDLLIMPORT int max_worker_processes;
 extern PGDLLIMPORT int max_parallel_workers;
 
+extern PGDLLIMPORT int multixact_local_cache_entries;
+
 extern PGDLLIMPORT int MyProcPid;
 extern PGDLLIMPORT pg_time_t MyStartTime;
 extern PGDLLIMPORT struct Port *MyProcPort;
-- 
2.24.3 (Apple Git-128)



  [application/octet-stream] v1106-0003-Add-conditional-variable-to-wait-for-next-Mult.patch (4.4K, ../[email protected]/4-v1106-0003-Add-conditional-variable-to-wait-for-next-Mult.patch)
  download | inline diff:
From 38dc550f363c1c8878fbf06d5616614a36112fac Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Mon, 26 Oct 2020 01:55:31 +0300
Subject: [PATCH v1106 3/4] Add conditional variable to wait for next MultXact
 offset in corner case

GetMultiXactIdMembers() has a corner case, when the next multixact offset is
not yet set.  In this case GetMultiXactIdMembers() has to sleep till this offset
is set.  Currently the sleeping is implemented in naive way using pg_sleep()
and retry.  This commit implements sleeping with conditional variable, which
provides more efficient way for waiting till the event.

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/multixact.c | 35 ++++++++++++++++++++++++--
 src/backend/postmaster/pgstat.c        |  3 +++
 src/include/pgstat.h                   |  3 ++-
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 6203be0aa3..5d2bbb1ca6 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,13 @@ typedef struct MultiXactStateData
 	/* support for members anti-wraparound measures */
 	MultiXactOffset offsetStopLimit;	/* known if oldestOffsetKnown */
 
+	/*
+	 * Conditional variable for waiting till the filling of the next multixact
+	 * will be finished.  See GetMultiXactIdMembers() and RecordNewMultiXact()
+	 * for details.
+	 */
+	ConditionVariable nextoffCV;
+
 	/*
 	 * Per-backend data starts here.  We have two arrays stored in the area
 	 * immediately following the MultiXactStateData struct. Each is indexed by
@@ -871,6 +879,14 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
 	/* Exchange our lock */
 	LWLockRelease(MultiXactOffsetControlLock);
 
+	/*
+	 * 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->nextoffCV);
+
 	LWLockAcquire(MultiXactMemberControlLock, LW_EXCLUSIVE);
 
 	prev_pageno = -1;
@@ -1368,9 +1384,23 @@ 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->nextoffCV);
+
+			/*
+			 * We don't have to recheck if multixact was filled in during
+			 * ConditionVariablePrepareToSleep(), because we were holding
+			 * MultiXactOffsetSLRULock.
+			 */
 			LWLockRelease(MultiXactOffsetControlLock);
-			CHECK_FOR_INTERRUPTS();
-			pg_usleep(1000L);
+
+			ConditionVariableSleep(&MultiXactState->nextoffCV,
+								   WAIT_EVENT_WAIT_NEXT_MXMEMBERS);
+			ConditionVariableCancelSleep();
 			goto retry;
 		}
 
@@ -1849,6 +1879,7 @@ MultiXactShmemInit(void)
 
 		/* Make sure we zero out the per-backend state */
 		MemSet(MultiXactState, 0, SHARED_MULTIXACT_STATE_SIZE);
+		ConditionVariableInit(&MultiXactState->nextoffCV);
 	}
 	else
 		Assert(found);
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 4895354a28..8a8b9a4eef 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3730,6 +3730,9 @@ pgstat_get_wait_ipc(WaitEventIPC w)
 		case WAIT_EVENT_SYNC_REP:
 			event_name = "SyncRep";
 			break;
+		case WAIT_EVENT_WAIT_NEXT_MXMEMBERS:
+			event_name = "MultiXactWaitNextMembers";
+			break;
 			/* no default case, so that compiler will warn */
 	}
 
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 58e2e71c6f..e89d946669 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -832,7 +832,8 @@ typedef enum
 	WAIT_EVENT_REPLICATION_ORIGIN_DROP,
 	WAIT_EVENT_REPLICATION_SLOT_DROP,
 	WAIT_EVENT_SAFE_SNAPSHOT,
-	WAIT_EVENT_SYNC_REP
+	WAIT_EVENT_SYNC_REP,
+	WAIT_EVENT_WAIT_NEXT_MXMEMBERS
 } WaitEventIPC;
 
 /* ----------
-- 
2.24.3 (Apple Git-128)



  [application/octet-stream] v1106-0004-Add-GUCs-to-tune-MultiXact-SLRUs.patch (6.2K, ../[email protected]/5-v1106-0004-Add-GUCs-to-tune-MultiXact-SLRUs.patch)
  download | inline diff:
From e273a508592236dbf5c60417b5878961c2483bca Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Sat, 9 May 2020 16:42:07 +0500
Subject: [PATCH v1106 4/4] Add GUCs to tune MultiXact SLRUs

---
 doc/src/sgml/config.sgml               | 31 ++++++++++++++++++++++++++
 src/backend/access/transam/multixact.c |  8 +++----
 src/backend/utils/init/globals.c       |  2 ++
 src/backend/utils/misc/guc.c           | 22 ++++++++++++++++++
 src/include/access/multixact.h         |  4 ----
 src/include/miscadmin.h                |  2 ++
 6 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 15fb2f2bde..44ebacc713 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1673,6 +1673,37 @@ include_dir 'conf.d'
        </para>
       </listitem>
      </varlistentry>
+     
+    <varlistentry id="guc-multixact-offsets-slru-buffers" xreflabel="multixact_offsets_slru_buffers">
+      <term><varname>multixact_offsets_slru_buffers</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>multixact_offsets_slru_buffers</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the amount of memory to be used for MultiXact offsets. MultiXact offsets
+        are used to store informaion about offsets of multiple row lockers (caused by SELECT FOR UPDATE and others).
+        It defaults to 64 kilobytes (<literal>64KB</literal>).
+       </para>
+      </listitem>
+     </varlistentry>
+
+    <varlistentry id="guc-multixact-members-slru-buffers" xreflabel="multixact_members_slru_buffers">
+      <term><varname>multixact_members_slru_buffers</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>multixact_members_slru_buffers</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the amount of memory to be used for MultiXact members. MultiXact members
+        are used to store informaion about XIDs of multiple row lockers. Tipically <varname>multixact_members_slru_buffers</varname>
+        is twice more than <varname>multixact_offsets_slru_buffers</varname>.
+        It defaults to 128 kilobytes (<literal>128KB</literal>).
+       </para>
+      </listitem>
+     </varlistentry>
 
      <varlistentry id="guc-max-stack-depth" xreflabel="max_stack_depth">
       <term><varname>max_stack_depth</varname> (<type>integer</type>)
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 5d2bbb1ca6..31324714b8 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1844,8 +1844,8 @@ MultiXactShmemSize(void)
 			 mul_size(sizeof(MultiXactId) * 2, MaxOldestSlot))
 
 	size = SHARED_MULTIXACT_STATE_SIZE;
-	size = add_size(size, SimpleLruShmemSize(NUM_MXACTOFFSET_BUFFERS, 0));
-	size = add_size(size, SimpleLruShmemSize(NUM_MXACTMEMBER_BUFFERS, 0));
+	size = add_size(size, SimpleLruShmemSize(multixact_offsets_slru_buffers, 0));
+	size = add_size(size, SimpleLruShmemSize(multixact_members_slru_buffers, 0));
 
 	return size;
 }
@@ -1861,11 +1861,11 @@ MultiXactShmemInit(void)
 	MultiXactMemberCtl->PagePrecedes = MultiXactMemberPagePrecedes;
 
 	SimpleLruInit(MultiXactOffsetCtl,
-				  "multixact_offset", NUM_MXACTOFFSET_BUFFERS, 0,
+				  "multixact_offset", multixact_offsets_slru_buffers, 0,
 				  MultiXactOffsetControlLock, "pg_multixact/offsets",
 				  LWTRANCHE_MXACTOFFSET_BUFFERS);
 	SimpleLruInit(MultiXactMemberCtl,
-				  "multixact_member", NUM_MXACTMEMBER_BUFFERS, 0,
+				  "multixact_member", multixact_members_slru_buffers, 0,
 				  MultiXactMemberControlLock, "pg_multixact/members",
 				  LWTRANCHE_MXACTMEMBER_BUFFERS);
 
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 22af834150..5a8517f8eb 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -149,3 +149,5 @@ bool		VacuumCostActive = false;
 double		vacuum_cleanup_index_scale_factor;
 
 int         multixact_local_cache_entries = 256;
+int			multixact_offsets_slru_buffers = 8;
+int			multixact_members_slru_buffers = 16;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index b54a063782..578452d757 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2036,6 +2036,28 @@ static struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"multixact_offsets_slru_buffers", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("Sets the number of shared memory buffers used for MultiXact offsets SLRU."),
+			NULL,
+			GUC_UNIT_BLOCKS
+		},
+		&multixact_offsets_slru_buffers,
+		8, 2, INT_MAX / 2,
+		NULL, NULL, NULL
+	},
+
+	{
+		{"multixact_members_slru_buffers", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("Sets the number of shared memory buffers used for MultiXact members SLRU."),
+			NULL,
+			GUC_UNIT_BLOCKS
+		},
+		&multixact_members_slru_buffers,
+		16, 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/access/multixact.h b/src/include/access/multixact.h
index 18fe380c5f..6e14fb7b29 100644
--- a/src/include/access/multixact.h
+++ b/src/include/access/multixact.h
@@ -28,10 +28,6 @@
 
 #define MaxMultiXactOffset	((MultiXactOffset) 0xFFFFFFFF)
 
-/* Number of SLRU buffers to use for multixact */
-#define NUM_MXACTOFFSET_BUFFERS		8
-#define NUM_MXACTMEMBER_BUFFERS		16
-
 /*
  * Possible multixact lock modes ("status").  The first four modes are for
  * tuple locks (FOR KEY SHARE, FOR SHARE, FOR NO KEY UPDATE, FOR UPDATE); the
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 11c3c8e6c5..e3fd0a264d 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -160,6 +160,8 @@ extern PGDLLIMPORT int MaxBackends;
 extern PGDLLIMPORT int MaxConnections;
 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;
 
-- 
2.24.3 (Apple Git-128)



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], [email protected], [email protected], [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