public inbox for [email protected]  
help / color / mirror / Atom feed
Re: MultiXact\SLRU buffers configuration
18+ messages / 11 participants
[nested] [flat]

* Re: MultiXact\SLRU buffers configuration
@ 2022-02-19 21:42  Andres Freund <[email protected]>
  0 siblings, 1 reply; 18+ messages in thread

From: Andres Freund @ 2022-02-19 21:42 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Andrey Borodin <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

On 2022-02-20 10:38:53 +1300, Thomas Munro wrote:
> Back to this patch: assuming we can settle on a good-enough-for-now
> replacement algorithm, do we want to add this set of 7 GUCs?  Does
> anyone else want to weigh in on that?

I'm -0.2 on it, given that we have a better path forward.






^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2022-02-20 07:35  Andrey Borodin <[email protected]>
  parent: Andres Freund <[email protected]>
  0 siblings, 1 reply; 18+ messages in thread

From: Andrey Borodin @ 2022-02-20 07:35 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers



> On 20 Feb 2022, at 02:42, Andres Freund <[email protected]> wrote:
> 
> On 2022-02-20 10:38:53 +1300, Thomas Munro wrote:
>> Back to this patch: assuming we can settle on a good-enough-for-now
>> replacement algorithm, do we want to add this set of 7 GUCs?  Does
>> anyone else want to weigh in on that?
> 
> I'm -0.2 on it, given that we have a better path forward.
That’s a really good path forward, but it's discussed at least for 3.5 years[0]. And guaranteed not to be there until 2023. Gilles, Shawn, Dmitry expressed their opinion in lines with that the patch “is a must-have” referring to real pathological performance degradation inflicted by SLRU cache starvation. And I can remember dozen of other incidents that would not happen if the patch was applied, e.g. this post is referring to the patch as a cure [1].

Best regards, Andrey Borodin.

[0] https://www.postgresql.org/message-id/flat/20180814213500.GA74618%4060f81dc409fc.ant.amazon.com
[1] https://about.gitlab.com/blog/2021/09/29/why-we-spent-the-last-month-eliminating-postgresql-subtrans...	





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2022-07-21 13:00  Yura Sokolov <[email protected]>
  parent: Andrey Borodin <[email protected]>
  0 siblings, 1 reply; 18+ messages in thread

From: Yura Sokolov @ 2022-07-21 13:00 UTC (permalink / raw)
  To: Andrey Borodin <[email protected]>; Andres Freund <[email protected]>; +Cc: Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

Good day, all.

I did benchmark of patch on 2 socket Xeon 5220 CPU @ 2.20GHz .
I used "benchmark" used to reproduce problems with SLRU on our
customers setup.
In opposite to Shawn's tests I concentrated on bad case: a lot
of contention.

slru-funcs.sql - function definitions
  - functions creates a lot of subtrunsactions to stress subtrans
  - and select random rows for share to stress multixacts

slru-call.sql - function call for benchmark

slru-ballast.sql - randomly select 1000 consequent rows
    "for update skip locked" to stress multixacts

patch1 - make SLRU buffers configurable
patch2 - make "8-associative banks"

Benchmark done by pgbench.
Inited with scale 1 to induce contention.
    pgbench -i -s 1 testdb

Benchmark 1:
- low number of connections (50), 60% slru-call, 40% slru-ballast
    pgbench -f slru-call.sql@60 -f slru-ballast.sql@40 -c 50 -j 75 -P 1 -T 30 testdb

version | subtrans | multixact | tps
        | buffers  | offs/memb | func+ballast
--------+----------+-----------+------
master  | 32       | 8/16      | 184+119
patch1  | 32       | 8/16      | 184+119
patch1  | 1024     | 8/16      | 121+77
patch1  | 1024     | 512/1024  | 118+75
patch2  | 32       | 8/16      | 190+122
patch2  | 1024     | 8/16      | 190+125
patch2  | 1024     | 512/1024  | 190+127

As you see, this test case degrades with dumb increase of
SLRU buffers. But use of "hash table" in form of "associative
buckets" makes performance stable.

Benchmark 2:
- high connection number (600), 98% slru-call, 2% slru-ballast
    pgbench -f slru-call.sql@98 -f slru-ballast.sql@2 -c 600 -j 75 -P 1 -T 30 testdb

I don't paste "ballast" tps here since 2% make them too small,
and they're very noisy.

version | subtrans | multixact | tps
        | buffers  | offs/memb | func
--------+----------+-----------+------
master  | 32       | 8/16      | 13
patch1  | 32  
    | 8/16      | 13
patch1  | 1024     | 8/16      | 31
patch1  | 1024     | 512/1024  | 53
patch2  | 32       | 8/16      | 12
patch2  | 1024     | 8/16      | 34
patch2  | 1024     | 512/1024  | 67

In this case simple buffer increase does help. But "buckets"
increase performance gain.

I didn't paste here results third part of patch ("Pack SLRU...")
because I didn't see any major performance gain from it, and
it consumes large part of patch diff.

Rebased versions of first two patch parts are attached.

regards,

Yura Sokolov


Attachments:

  [application/sql] slru-ballast.sql (123B, ../../[email protected]/2-slru-ballast.sql)
  download

  [application/sql] slru-call.sql (25B, ../../[email protected]/3-slru-call.sql)
  download

  [application/sql] slru-func.sql (1.2K, ../../[email protected]/4-slru-func.sql)
  download

  [text/x-patch] v21-0002-Divide-SLRU-buffers-into-8-associative-banks.patch (4.9K, ../../[email protected]/5-v21-0002-Divide-SLRU-buffers-into-8-associative-banks.patch)
  download | inline diff:
From 41ec9d1c54184c515d53ecc8021c4a998813f2a9 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Sun, 11 Apr 2021 21:18:10 +0300
Subject: [PATCH v21 2/2] Divide SLRU buffers into 8-associative banks

We want to eliminate linear search within SLRU buffers.
To do so we divide SLRU buffers into banks. Each bank holds
approximately 8 buffers. Each SLRU pageno may reside only in one bank.
Adjacent pagenos reside in different banks.
---
 src/backend/access/transam/slru.c | 43 ++++++++++++++++++++++++++++---
 src/include/access/slru.h         |  2 ++
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index b65cb49d7ff..abc534bbd06 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -134,7 +134,7 @@ typedef enum
 static SlruErrorCause slru_errcause;
 static int	slru_errno;
 
-
+static void SlruAdjustNSlots(int* nslots, int* banksize, int* bankoffset);
 static void SimpleLruZeroLSNs(SlruCtl ctl, int slotno);
 static void SimpleLruWaitIO(SlruCtl ctl, int slotno);
 static void SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata);
@@ -148,6 +148,30 @@ static bool SlruScanDirCbDeleteCutoff(SlruCtl ctl, char *filename,
 									  int segpage, void *data);
 static void SlruInternalDeleteSegment(SlruCtl ctl, int segno);
 
+/*
+ * Pick bank size optimal for N-assiciative SLRU buffers.
+ * We expect bank number to be picked from lowest bits of requested pageno.
+ * Thus we want number of banks to be power of 2. This routine computes number
+ * of banks aiming to make each bank of size 8. So we can pack page number and
+ * statuses of each bank on one cacheline.
+ */
+static void SlruAdjustNSlots(int* nslots, int* banksize, int* bankoffset)
+{
+	int nbanks = 1;
+	*banksize = *nslots;
+	*bankoffset = 0;
+	while (*banksize > 15)
+	{
+		if ((*banksize & 1) != 0)
+			*banksize +=1;
+		*banksize /= 2;
+		nbanks *= 2;
+		*bankoffset += 1;
+	}
+	elog(DEBUG5, "nslots %d banksize %d nbanks %d ", *nslots, *banksize, nbanks);
+	*nslots = *banksize * nbanks;
+}
+
 /*
  * Initialization of shared memory
  */
@@ -156,6 +180,8 @@ Size
 SimpleLruShmemSize(int nslots, int nlsns)
 {
 	Size		sz;
+	int bankoffset, banksize;
+	SlruAdjustNSlots(&nslots, &banksize, &bankoffset);
 
 	/* we assume nslots isn't so large as to risk overflow */
 	sz = MAXALIGN(sizeof(SlruSharedData));
@@ -190,6 +216,8 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 {
 	SlruShared	shared;
 	bool		found;
+	int bankoffset, banksize;
+	SlruAdjustNSlots(&nslots, &banksize, &bankoffset);
 
 	shared = (SlruShared) ShmemInitStruct(name,
 										  SimpleLruShmemSize(nslots, nlsns),
@@ -209,6 +237,9 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 		shared->ControlLock = ctllock;
 
 		shared->num_slots = nslots;
+		shared->bank_mask =  (1 << bankoffset) - 1;
+		shared->bank_size = banksize;
+		
 		shared->lsn_groups_per_page = nlsns;
 
 		shared->cur_lru_count = 0;
@@ -496,12 +527,14 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid)
 {
 	SlruShared	shared = ctl->shared;
 	int			slotno;
+	int			bankstart = (pageno & shared->bank_mask) * shared->bank_size;
+	int			bankend = bankstart + shared->bank_size;
 
 	/* Try to find the page while holding only shared lock */
 	LWLockAcquire(shared->ControlLock, LW_SHARED);
 
 	/* See if page is already in a buffer */
-	for (slotno = 0; slotno < shared->num_slots; slotno++)
+	for (slotno = bankstart; slotno < bankend; slotno++)
 	{
 		if (shared->page_number[slotno] == pageno &&
 			shared->page_status[slotno] != SLRU_PAGE_EMPTY &&
@@ -1030,7 +1063,9 @@ SlruSelectLRUPage(SlruCtl ctl, int pageno)
 		int			best_invalid_page_number = 0;	/* keep compiler quiet */
 
 		/* See if page already has a buffer assigned */
-		for (slotno = 0; slotno < shared->num_slots; slotno++)
+		int bankstart = (pageno & shared->bank_mask) * shared->bank_size;
+		int bankend = bankstart + shared->bank_size;
+		for (slotno = bankstart; slotno < bankend; slotno++)
 		{
 			if (shared->page_number[slotno] == pageno &&
 				shared->page_status[slotno] != SLRU_PAGE_EMPTY)
@@ -1065,7 +1100,7 @@ SlruSelectLRUPage(SlruCtl ctl, int pageno)
 		 * multiple pages with the same lru_count.
 		 */
 		cur_count = (shared->cur_lru_count)++;
-		for (slotno = 0; slotno < shared->num_slots; slotno++)
+		for (slotno = bankstart; slotno < bankend; slotno++)
 		{
 			int			this_delta;
 			int			this_page_number;
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index b7e2e2b55e3..cc9cc10d271 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -61,6 +61,8 @@ typedef struct SlruSharedData
 
 	/* Number of buffers managed by this SLRU structure */
 	int			num_slots;
+	int			bank_size;
+	int			bank_mask;
 
 	/*
 	 * Arrays holding info for each buffer slot.  Page number is undefined
-- 
2.30.2



  [text/x-patch] v21-0001-Make-all-SLRU-buffer-sizes-configurable.patch (25.7K, ../../[email protected]/6-v21-0001-Make-all-SLRU-buffer-sizes-configurable.patch)
  download | inline diff:
From e5f03ebac800f06f45c9ecb5f7139c96032e7fa7 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Mon, 15 Feb 2021 21:51:56 +0500
Subject: [PATCH v21 1/2] Make all SLRU buffer sizes configurable.

Provide new GUCs to set the number of buffers, instead of using hard
coded defaults.

Remove the limits on xact_buffers and commit_ts_buffers.  The default
sizes for those caches are ~0.2% and ~0.1% of shared_buffers, as before,
but now there is no cap at 128 and 16 buffers respectively (unless
track_commit_timestamp is disabled, in the latter case, then we might as
well keep it tiny).  Sizes much larger than the old limits have been
shown to be useful on modern systems, and an earlier commit replaced a
linear search with a hash table to avoid problems with extreme cases.

Author: Andrey M. Borodin <[email protected]>
Reviewed-by: Anastasia Lubennikova <[email protected]>
Reviewed-by: Tomas Vondra <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Reviewed-by: Gilles Darold <[email protected]>
Reviewed-by: Thomas Munro <[email protected]>
Discussion: https://postgr.es/m/2BEC2B3F-9B61-4C1D-9FB5-5FAB0F05EF86%40yandex-team.ru
---
 doc/src/sgml/config.sgml                      | 135 ++++++++++++++++++
 src/backend/access/transam/clog.c             |  23 ++-
 src/backend/access/transam/commit_ts.c        |   5 +
 src/backend/access/transam/multixact.c        |   8 +-
 src/backend/access/transam/subtrans.c         |   5 +-
 src/backend/commands/async.c                  |   8 +-
 src/backend/storage/lmgr/predicate.c          |   4 +-
 src/backend/utils/init/globals.c              |   8 ++
 src/backend/utils/misc/guc.c                  |  99 +++++++++++++
 src/backend/utils/misc/postgresql.conf.sample |   9 ++
 src/include/access/clog.h                     |  10 ++
 src/include/access/commit_ts.h                |   1 -
 src/include/access/multixact.h                |   4 -
 src/include/access/slru.h                     |   5 +
 src/include/access/subtrans.h                 |   2 -
 src/include/commands/async.h                  |   5 -
 src/include/miscadmin.h                       |   7 +
 src/include/storage/predicate.h               |   4 -
 18 files changed, 299 insertions(+), 43 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e2d728e0c4f..991dba69757 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1961,6 +1961,141 @@ include_dir 'conf.d'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-multixact-offsets-buffers" xreflabel="multixact_offsets_buffers">
+      <term><varname>multixact_offsets_buffers</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>multixact_offsets_buffers</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the amount of shared memory to use to cache the contents
+        of <literal>pg_multixact/offsets</literal> (see
+        <xref linkend="pgdata-contents-table"/>).
+        If this value is specified without units, it is taken as blocks,
+        that is <symbol>BLCKSZ</symbol> bytes, typically 8kB.
+        The default value is <literal>8</literal>.
+        This parameter can only be set at server start.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-multixact-members-buffers" xreflabel="multixact_members_buffers">
+      <term><varname>multixact_members_buffers</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>multixact_members_buffers</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the amount of shared memory to use to cache the contents
+        of <literal>pg_multixact/members</literal> (see
+        <xref linkend="pgdata-contents-table"/>).
+        If this value is specified without units, it is taken as blocks,
+        that is <symbol>BLCKSZ</symbol> bytes, typically 8kB.
+        The default value is <literal>16</literal>.
+        This parameter can only be set at server start.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-subtrans-buffers" xreflabel="subtrans_buffers">
+      <term><varname>subtrans_buffers</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>subtrans_buffers</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the amount of shared memory to use to cache the contents
+        of <literal>pg_subtrans</literal> (see
+        <xref linkend="pgdata-contents-table"/>).
+        If this value is specified without units, it is taken as blocks,
+        that is <symbol>BLCKSZ</symbol> bytes, typically 8kB.
+        The default value is <literal>8</literal>.
+        This parameter can only be set at server start.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-notify-buffers" xreflabel="notify_buffers">
+      <term><varname>notify_buffers</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>notify_buffers</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the amount of shared memory to use to cache the contents
+        of <literal>pg_notify</literal> (see
+        <xref linkend="pgdata-contents-table"/>).
+        If this value is specified without units, it is taken as blocks,
+        that is <symbol>BLCKSZ</symbol> bytes, typically 8kB.
+        The default value is <literal>8</literal>.
+        This parameter can only be set at server start.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-serial-buffers" xreflabel="serial_buffers">
+      <term><varname>serial_buffers</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>serial_buffers</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the amount of shared memory to use to cache the contents
+        of <literal>pg_serial</literal> (see
+        <xref linkend="pgdata-contents-table"/>).
+        If this value is specified without units, it is taken as blocks,
+        that is <symbol>BLCKSZ</symbol> bytes, typically 8kB.
+        The default value is <literal>16</literal>.
+        This parameter can only be set at server start.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-xact-buffers" xreflabel="xact_buffers">
+      <term><varname>xact_buffers</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>xact_buffers</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the amount of shared memory to use to cache the contents
+        of <literal>pg_xact</literal> (see
+        <xref linkend="pgdata-contents-table"/>).
+        If this value is specified without units, it is taken as blocks,
+        that is <symbol>BLCKSZ</symbol> bytes, typically 8kB.
+        The default value is <literal>0</literal>, which requests
+        <varname>shared_buffers</varname> / 512, but not fewer than 4 blocks.
+        This parameter can only be set at server start.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-commit-ts-buffers" xreflabel="commit_ts_buffers">
+      <term><varname>commit_ts_buffers</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>commit_ts_buffers</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies the amount of memory to use to cache the cotents of
+        <literal>pg_commit_ts</literal> (see
+        <xref linkend="pgdata-contents-table"/>).
+        If this value is specified without units, it is taken as blocks,
+        that is <symbol>BLCKSZ</symbol> bytes, typically 8kB.
+        The default value is <literal>0</literal>, which requests
+        <varname>shared_buffers</varname> / 1024, but not fewer than 4 blocks.
+        This parameter can only be set at server start.
+       </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/clog.c b/src/backend/access/transam/clog.c
index 3d9088a7048..e96a75d6959 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -58,8 +58,8 @@
 
 /* We need two bits per xact, so four xacts fit in a byte */
 #define CLOG_BITS_PER_XACT	2
-#define CLOG_XACTS_PER_BYTE 4
-#define CLOG_XACTS_PER_PAGE (BLCKSZ * CLOG_XACTS_PER_BYTE)
+StaticAssertDecl((CLOG_BITS_PER_XACT * CLOG_XACTS_PER_BYTE) == BITS_PER_BYTE,
+				 "CLOG_BITS_PER_XACT and CLOG_XACTS_PER_BYTE are inconsistent");
 #define CLOG_XACT_BITMASK	((1 << CLOG_BITS_PER_XACT) - 1)
 
 #define TransactionIdToPage(xid)	((xid) / (TransactionId) CLOG_XACTS_PER_PAGE)
@@ -665,23 +665,16 @@ TransactionIdGetStatus(TransactionId xid, XLogRecPtr *lsn)
 /*
  * Number of shared CLOG buffers.
  *
- * On larger multi-processor systems, it is possible to have many CLOG page
- * requests in flight at one time which could lead to disk access for CLOG
- * page if the required page is not found in memory.  Testing revealed that we
- * can get the best performance by having 128 CLOG buffers, more than that it
- * doesn't improve performance.
- *
- * Unconditionally keeping the number of CLOG buffers to 128 did not seem like
- * a good idea, because it would increase the minimum amount of shared memory
- * required to start, which could be a problem for people running very small
- * configurations.  The following formula seems to represent a reasonable
- * compromise: people with very low values for shared_buffers will get fewer
- * CLOG buffers as well, and everyone else will get 128.
+ * By default, we'll use 2MB of for every 1GB of shared buffers, up to the
+ * theoretical maximum useful value, but always at least 4 buffers.
  */
 Size
 CLOGShmemBuffers(void)
 {
-	return Min(128, Max(4, NBuffers / 512));
+	/* Use configured value if provided. */
+	if (xact_buffers > 0)
+		return Max(4, xact_buffers);
+	return Min(CLOG_MAX_ALLOWED_BUFFERS, Max(4, NBuffers / 512));
 }
 
 /*
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 4dc8d402bd3..48ca5007475 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -514,10 +514,15 @@ pg_xact_commit_timestamp_origin(PG_FUNCTION_ARGS)
  * We use a very similar logic as for the number of CLOG buffers (except we
  * scale up twice as fast with shared buffers, and the maximum is twice as
  * high); see comments in CLOGShmemBuffers.
+ * By default, we'll use 1MB of for every 1GB of shared buffers, up to the
+ * maximum value that slru.c will allow, but always at least 4 buffers.
  */
 Size
 CommitTsShmemBuffers(void)
 {
+	/* Use configured value if provided. */
+	if (commit_ts_buffers > 0)
+		return Max(4, commit_ts_buffers);
 	return Min(256, Max(4, NBuffers / 256));
 }
 
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 8f7d12950e5..71d17e338e9 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1834,8 +1834,8 @@ MultiXactShmemSize(void)
 			 mul_size(sizeof(MultiXactId) * 2, MaxOldestSlot))
 
 	size = SHARED_MULTIXACT_STATE_SIZE;
-	size = add_size(size, SimpleLruShmemSize(NUM_MULTIXACTOFFSET_BUFFERS, 0));
-	size = add_size(size, SimpleLruShmemSize(NUM_MULTIXACTMEMBER_BUFFERS, 0));
+	size = add_size(size, SimpleLruShmemSize(multixact_offsets_buffers, 0));
+	size = add_size(size, SimpleLruShmemSize(multixact_members_buffers, 0));
 
 	return size;
 }
@@ -1851,13 +1851,13 @@ MultiXactShmemInit(void)
 	MultiXactMemberCtl->PagePrecedes = MultiXactMemberPagePrecedes;
 
 	SimpleLruInit(MultiXactOffsetCtl,
-				  "MultiXactOffset", NUM_MULTIXACTOFFSET_BUFFERS, 0,
+				  "MultiXactOffset", multixact_offsets_buffers, 0,
 				  MultiXactOffsetSLRULock, "pg_multixact/offsets",
 				  LWTRANCHE_MULTIXACTOFFSET_BUFFER,
 				  SYNC_HANDLER_MULTIXACT_OFFSET);
 	SlruPagePrecedesUnitTests(MultiXactOffsetCtl, MULTIXACT_OFFSETS_PER_PAGE);
 	SimpleLruInit(MultiXactMemberCtl,
-				  "MultiXactMember", NUM_MULTIXACTMEMBER_BUFFERS, 0,
+				  "MultiXactMember", multixact_offsets_buffers, 0,
 				  MultiXactMemberSLRULock, "pg_multixact/members",
 				  LWTRANCHE_MULTIXACTMEMBER_BUFFER,
 				  SYNC_HANDLER_MULTIXACT_MEMBER);
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index 66d35481552..862b0eab966 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -31,6 +31,7 @@
 #include "access/slru.h"
 #include "access/subtrans.h"
 #include "access/transam.h"
+#include "miscadmin.h"
 #include "pg_trace.h"
 #include "utils/snapmgr.h"
 
@@ -184,14 +185,14 @@ SubTransGetTopmostTransaction(TransactionId xid)
 Size
 SUBTRANSShmemSize(void)
 {
-	return SimpleLruShmemSize(NUM_SUBTRANS_BUFFERS, 0);
+	return SimpleLruShmemSize(subtrans_buffers, 0);
 }
 
 void
 SUBTRANSShmemInit(void)
 {
 	SubTransCtl->PagePrecedes = SubTransPagePrecedes;
-	SimpleLruInit(SubTransCtl, "Subtrans", NUM_SUBTRANS_BUFFERS, 0,
+	SimpleLruInit(SubTransCtl, "Subtrans", subtrans_buffers, 0,
 				  SubtransSLRULock, "pg_subtrans",
 				  LWTRANCHE_SUBTRANS_BUFFER, SYNC_HANDLER_NONE);
 	SlruPagePrecedesUnitTests(SubTransCtl, SUBTRANS_XACTS_PER_PAGE);
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 3e1b92df030..6b980603302 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -117,7 +117,7 @@
  * frontend during startup.)  The above design guarantees that notifies from
  * other backends will never be missed by ignoring self-notifies.
  *
- * The amount of shared memory used for notify management (NUM_NOTIFY_BUFFERS)
+ * The amount of shared memory used for notify management (notify_buffers)
  * can be varied without affecting anything but performance.  The maximum
  * amount of notification data that can be queued at one time is determined
  * by slru.c's wraparound limit; see QUEUE_MAX_PAGE below.
@@ -235,7 +235,7 @@ typedef struct QueuePosition
  *
  * Resist the temptation to make this really large.  While that would save
  * work in some places, it would add cost in others.  In particular, this
- * should likely be less than NUM_NOTIFY_BUFFERS, to ensure that backends
+ * should likely be less than notify_buffers, to ensure that backends
  * catch up before the pages they'll need to read fall out of SLRU cache.
  */
 #define QUEUE_CLEANUP_DELAY 4
@@ -521,7 +521,7 @@ AsyncShmemSize(void)
 	size = mul_size(MaxBackends + 1, sizeof(QueueBackendStatus));
 	size = add_size(size, offsetof(AsyncQueueControl, backend));
 
-	size = add_size(size, SimpleLruShmemSize(NUM_NOTIFY_BUFFERS, 0));
+	size = add_size(size, SimpleLruShmemSize(notify_buffers, 0));
 
 	return size;
 }
@@ -569,7 +569,7 @@ AsyncShmemInit(void)
 	 * Set up SLRU management of the pg_notify data.
 	 */
 	NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
-	SimpleLruInit(NotifyCtl, "Notify", NUM_NOTIFY_BUFFERS, 0,
+	SimpleLruInit(NotifyCtl, "Notify", notify_buffers, 0,
 				  NotifySLRULock, "pg_notify", LWTRANCHE_NOTIFY_BUFFER,
 				  SYNC_HANDLER_NONE);
 
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 5136da6ea36..54a0d66d579 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -872,7 +872,7 @@ SerialInit(void)
 	 */
 	SerialSlruCtl->PagePrecedes = SerialPagePrecedesLogically;
 	SimpleLruInit(SerialSlruCtl, "Serial",
-				  NUM_SERIAL_BUFFERS, 0, SerialSLRULock, "pg_serial",
+				  serial_buffers, 0, SerialSLRULock, "pg_serial",
 				  LWTRANCHE_SERIAL_BUFFER, SYNC_HANDLER_NONE);
 #ifdef USE_ASSERT_CHECKING
 	SerialPagePrecedesLogicallyUnitTests();
@@ -1396,7 +1396,7 @@ PredicateLockShmemSize(void)
 
 	/* Shared memory structures for SLRU tracking of old committed xids. */
 	size = add_size(size, sizeof(SerialControlData));
-	size = add_size(size, SimpleLruShmemSize(NUM_SERIAL_BUFFERS, 0));
+	size = add_size(size, SimpleLruShmemSize(serial_buffers, 0));
 
 	return size;
 }
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 1a5d29ac9ba..cc0ca91a8b3 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -151,3 +151,11 @@ int64		VacuumPageDirty = 0;
 
 int			VacuumCostBalance = 0;	/* working state for vacuum */
 bool		VacuumCostActive = false;
+
+int			multixact_offsets_buffers = 8;
+int			multixact_members_buffers = 16;
+int			subtrans_buffers = 32;
+int			notify_buffers = 8;
+int			serial_buffers = 16;
+int			xact_buffers = 0;
+int			commit_ts_buffers = 0;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index af4a1c30689..2d9121b9265 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -32,9 +32,11 @@
 #endif
 #include <unistd.h>
 
+#include "access/clog.h"
 #include "access/commit_ts.h"
 #include "access/gin.h"
 #include "access/rmgr.h"
+#include "access/slru.h"
 #include "access/tableam.h"
 #include "access/toast_compression.h"
 #include "access/transam.h"
@@ -209,6 +211,8 @@ static const char *show_tcp_keepalives_idle(void);
 static const char *show_tcp_keepalives_interval(void);
 static const char *show_tcp_keepalives_count(void);
 static const char *show_tcp_user_timeout(void);
+static const char *show_xact_buffers(void);
+static const char *show_commit_ts_buffers(void);
 static bool check_maxconnections(int *newval, void **extra, GucSource source);
 static bool check_max_worker_processes(int *newval, void **extra, GucSource source);
 static bool check_autovacuum_max_workers(int *newval, void **extra, GucSource source);
@@ -2426,6 +2430,83 @@ static struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"multixact_offsets_buffers", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("Sets the number of shared memory buffers used for the MultiXact offset SLRU cache."),
+			NULL,
+			GUC_UNIT_BLOCKS
+		},
+		&multixact_offsets_buffers,
+		8, 2, SLRU_MAX_ALLOWED_BUFFERS,
+		NULL, NULL, NULL
+	},
+
+	{
+		{"multixact_members_buffers", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("Sets the number of shared memory buffers used for the MultiXact member SLRU cache."),
+			NULL,
+			GUC_UNIT_BLOCKS
+		},
+		&multixact_members_buffers,
+		16, 2, SLRU_MAX_ALLOWED_BUFFERS,
+		NULL, NULL, NULL
+	},
+
+	{
+		{"subtrans_buffers", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("Sets the number of shared memory buffers used for the sub-transaction SLRU cache."),
+			NULL,
+			GUC_UNIT_BLOCKS
+		},
+		&subtrans_buffers,
+		32, 2, SLRU_MAX_ALLOWED_BUFFERS,
+		NULL, NULL, NULL
+	},
+
+	{
+		{"notify_buffers", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("Sets the number of shared memory buffers used for the NOTIFY message SLRU cache."),
+			NULL,
+			GUC_UNIT_BLOCKS
+		},
+		&notify_buffers,
+		8, 2, SLRU_MAX_ALLOWED_BUFFERS,
+		NULL, NULL, NULL
+	},
+
+	{
+		{"serial_buffers", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("Sets the number of shared memory buffers used for the serializable transaction SLRU cache."),
+			NULL,
+			GUC_UNIT_BLOCKS
+		},
+		&serial_buffers,
+		16, 2, SLRU_MAX_ALLOWED_BUFFERS,
+		NULL, NULL, NULL
+	},
+
+	{
+		{"xact_buffers", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("Sets the number of shared memory buffers used for the transaction status SLRU cache."),
+			NULL,
+			GUC_UNIT_BLOCKS
+		},
+		&xact_buffers,
+		0, 0, CLOG_MAX_ALLOWED_BUFFERS,
+		NULL, NULL, show_xact_buffers
+	},
+
+	{
+		{"commit_ts_buffers", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("Sets the size of the dedicated buffer pool used for the commit timestamp SLRU cache."),
+			NULL,
+			GUC_UNIT_BLOCKS
+		},
+		&commit_ts_buffers,
+		0, 0, SLRU_MAX_ALLOWED_BUFFERS,
+		NULL, NULL, show_commit_ts_buffers
+	},
+
 	{
 		{"temp_buffers", PGC_USERSET, RESOURCES_MEM,
 			gettext_noop("Sets the maximum number of temporary buffers used by each session."),
@@ -12447,6 +12528,24 @@ show_tcp_user_timeout(void)
 	return nbuf;
 }
 
+static const char *
+show_xact_buffers(void)
+{
+	static char nbuf[16];
+
+	snprintf(nbuf, sizeof(nbuf), "%zu", CLOGShmemBuffers());
+	return nbuf;
+}
+
+static const char *
+show_commit_ts_buffers(void)
+{
+	static char nbuf[16];
+
+	snprintf(nbuf, sizeof(nbuf), "%zu", CommitTsShmemBuffers());
+	return nbuf;
+}
+
 static bool
 check_maxconnections(int *newval, void **extra, GucSource source)
 {
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index b4bc06e5f5a..2cb827e1e3b 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -195,6 +195,15 @@
 #old_snapshot_threshold = -1		# 1min-60d; -1 disables; 0 is immediate
 					# (change requires restart)
 
+# - SLRU Buffers (change requires restart) -
+
+#xact_buffers = 0			# memory for pg_xact (0 = auto)
+#subtrans_buffers = 32			# memory for pg_subtrans
+#multixact_offsets_buffers = 8		# memory for pg_multixact/offsets
+#multixact_members_buffers = 16		# memory for pg_multixact/members
+#notify_buffers = 8			# memory for pg_notify
+#serial_buffers = 16			# memory for pg_serial
+#commit_ts_buffers = 0			# memory for pg_commit_ts (0 = auto)
 
 #------------------------------------------------------------------------------
 # WRITE-AHEAD LOG
diff --git a/src/include/access/clog.h b/src/include/access/clog.h
index 543f2e2643a..17d103aa4da 100644
--- a/src/include/access/clog.h
+++ b/src/include/access/clog.h
@@ -15,6 +15,16 @@
 #include "storage/sync.h"
 #include "lib/stringinfo.h"
 
+/*
+ * Don't allow xact_buffers to be set higher than could possibly be useful or
+ * SLRU would allow.
+ */
+#define CLOG_XACTS_PER_BYTE 4
+#define CLOG_XACTS_PER_PAGE (BLCKSZ * CLOG_XACTS_PER_BYTE)
+#define CLOG_MAX_ALLOWED_BUFFERS \
+	Min(SLRU_MAX_ALLOWED_BUFFERS, \
+		(((MaxTransactionId / 2) + (CLOG_XACTS_PER_PAGE - 1)) / CLOG_XACTS_PER_PAGE))
+
 /*
  * Possible transaction statuses --- note that all-zeroes is the initial
  * state.
diff --git a/src/include/access/commit_ts.h b/src/include/access/commit_ts.h
index 7662f8e1a9c..d928dcc9352 100644
--- a/src/include/access/commit_ts.h
+++ b/src/include/access/commit_ts.h
@@ -16,7 +16,6 @@
 #include "replication/origin.h"
 #include "storage/sync.h"
 
-
 extern PGDLLIMPORT bool track_commit_timestamp;
 
 extern void TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h
index a5600a320ae..da7b8f0abb9 100644
--- a/src/include/access/multixact.h
+++ b/src/include/access/multixact.h
@@ -29,10 +29,6 @@
 
 #define MaxMultiXactOffset	((MultiXactOffset) 0xFFFFFFFF)
 
-/* Number of SLRU buffers to use for multixact */
-#define NUM_MULTIXACTOFFSET_BUFFERS		8
-#define NUM_MULTIXACTMEMBER_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/access/slru.h b/src/include/access/slru.h
index 130c41c8632..b7e2e2b55e3 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -17,6 +17,11 @@
 #include "storage/lwlock.h"
 #include "storage/sync.h"
 
+/*
+ * To avoid overflowing internal arithmetic and the size_t data type, the
+ * number of buffers should not exceed this number.
+ */
+#define SLRU_MAX_ALLOWED_BUFFERS ((1024 * 1024 * 1024) / BLCKSZ)
 
 /*
  * Define SLRU segment size.  A page is the same BLCKSZ as is used everywhere
diff --git a/src/include/access/subtrans.h b/src/include/access/subtrans.h
index f94e116640b..1ddb62883b0 100644
--- a/src/include/access/subtrans.h
+++ b/src/include/access/subtrans.h
@@ -11,8 +11,6 @@
 #ifndef SUBTRANS_H
 #define SUBTRANS_H
 
-/* Number of SLRU buffers to use for subtrans */
-#define NUM_SUBTRANS_BUFFERS	32
 
 extern void SubTransSetParent(TransactionId xid, TransactionId parent);
 extern TransactionId SubTransGetParent(TransactionId xid);
diff --git a/src/include/commands/async.h b/src/include/commands/async.h
index 926af933d1b..402d184b9b3 100644
--- a/src/include/commands/async.h
+++ b/src/include/commands/async.h
@@ -15,11 +15,6 @@
 
 #include <signal.h>
 
-/*
- * The number of SLRU page buffers we use for the notification queue.
- */
-#define NUM_NOTIFY_BUFFERS	8
-
 extern PGDLLIMPORT bool Trace_notify;
 extern PGDLLIMPORT volatile sig_atomic_t notifyInterruptPending;
 
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index ea9a56d3955..e65990e04e8 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -177,6 +177,13 @@ 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_buffers;
+extern PGDLLIMPORT int multixact_members_buffers;
+extern PGDLLIMPORT int subtrans_buffers;
+extern PGDLLIMPORT int notify_buffers;
+extern PGDLLIMPORT int serial_buffers;
+extern PGDLLIMPORT int xact_buffers;
+extern PGDLLIMPORT int commit_ts_buffers;
 
 extern PGDLLIMPORT int MyProcPid;
 extern PGDLLIMPORT pg_time_t MyStartTime;
diff --git a/src/include/storage/predicate.h b/src/include/storage/predicate.h
index 8dfcb3944b4..1e3f757ec30 100644
--- a/src/include/storage/predicate.h
+++ b/src/include/storage/predicate.h
@@ -26,10 +26,6 @@ extern PGDLLIMPORT int max_predicate_locks_per_xact;
 extern PGDLLIMPORT int max_predicate_locks_per_relation;
 extern PGDLLIMPORT int max_predicate_locks_per_page;
 
-
-/* Number of SLRU buffers to use for Serial SLRU */
-#define NUM_SERIAL_BUFFERS		16
-
 /*
  * A handle used for sharing SERIALIZABLEXACT objects between the participants
  * in a parallel query.
-- 
2.30.2



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2022-07-23 08:39  Andrey Borodin <[email protected]>
  parent: Yura Sokolov <[email protected]>
  0 siblings, 2 replies; 18+ messages in thread

From: Andrey Borodin @ 2022-07-23 08:39 UTC (permalink / raw)
  To: Yura Sokolov <[email protected]>; +Cc: Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers



> On 21 Jul 2022, at 18:00, Yura Sokolov <[email protected]> wrote:
> 
> In this case simple buffer increase does help. But "buckets"
> increase performance gain.
Yura, thank you for your benchmarks!
We already knew that patch can save the day on pathological workloads, now we have a proof of this.
Also there's the evidence that user can blindly increase size of SLRU if they want (with the second patch). So there's no need for hard explanations on how to tune the buffers size.

Thomas, do you still have any doubts? Or is it certain that SLRU will be replaced by any better subsystem in 16?


Best regards, Andrey Borodin.




^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2022-07-23 08:47  Thomas Munro <[email protected]>
  parent: Andrey Borodin <[email protected]>
  1 sibling, 1 reply; 18+ messages in thread

From: Thomas Munro @ 2022-07-23 08:47 UTC (permalink / raw)
  To: Andrey Borodin <[email protected]>; +Cc: Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

On Sat, Jul 23, 2022 at 8:41 PM Andrey Borodin <[email protected]> wrote:
> Thomas, do you still have any doubts? Or is it certain that SLRU will be replaced by any better subsystem in 16?

Hi Andrey,

Sorry for my lack of replies on this and the other SLRU thread -- I'm
thinking and experimenting.  More soon.





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2022-08-16 19:36  [email protected]
  parent: Andrey Borodin <[email protected]>
  1 sibling, 1 reply; 18+ messages in thread

From: [email protected] @ 2022-08-16 19:36 UTC (permalink / raw)
  To: Andrey Borodin <[email protected]>; +Cc: Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

> Andrey Borodin wrote 2022-07-23 11:39:
> 
> Yura, thank you for your benchmarks!
> We already knew that patch can save the day on pathological workloads,
> now we have a proof of this.
> Also there's the evidence that user can blindly increase size of SLRU
> if they want (with the second patch). So there's no need for hard
> explanations on how to tune the buffers size.

Hi @Andrey.Borodin, With some considerations and performance checks from 
@Yura.Sokolov we simplified your approach by the following:

1. Preamble. We feel free to increase any SLRU's, since there's no 
performance degradation on large Buffers count using your SLRU buckets 
solution.
2. `slru_buffers_size_scale` is only one config param introduced for all 
SLRUs. It scales SLRUs upper cap by power 2.
3. All SLRU buffers count are capped by both `MBuffers (shared_buffers)` 
and `slru_buffers_size_scale`. see
4. Magic initial constants `NUM_*_BUFFERS << slru_buffers_size_scale` 
are applied for every SLRU.
5. All SLRU buffers are always sized as power of 2, their hash bucket 
size is always 8.

There's attached patch for your consideration. It does gather and 
simplify both `v21-0001-Make-all-SLRU-buffer-sizes-configurable.patch` 
and `v21-0002-Divide-SLRU-buffers-into-8-associative-banks.patch` to 
much simpler approach.

Thank you, Yours,
- Ivan


Attachments:

  [text/x-diff] v22-0006-bucketed-SLRUs-simplified_patch.patch (18.0K, ../../[email protected]/2-v22-0006-bucketed-SLRUs-simplified_patch.patch)
  download | inline diff:
Index: src/include/miscadmin.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
--- a/src/include/miscadmin.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/miscadmin.h	(date 1660678108730)
@@ -176,6 +176,7 @@
 extern PGDLLIMPORT int MaxConnections;
 extern PGDLLIMPORT int max_worker_processes;
 extern PGDLLIMPORT int max_parallel_workers;
+extern PGDLLIMPORT int slru_buffers_size_scale;
 
 extern PGDLLIMPORT int MyProcPid;
 extern PGDLLIMPORT pg_time_t MyStartTime;
Index: src/include/access/subtrans.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/access/subtrans.h b/src/include/access/subtrans.h
--- a/src/include/access/subtrans.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/access/subtrans.h	(date 1660678108698)
@@ -12,7 +12,7 @@
 #define SUBTRANS_H
 
 /* Number of SLRU buffers to use for subtrans */
-#define NUM_SUBTRANS_BUFFERS	32
+#define NUM_SUBTRANS_BUFFERS	(32 << slru_buffers_size_scale)
 
 extern void SubTransSetParent(TransactionId xid, TransactionId parent);
 extern TransactionId SubTransGetParent(TransactionId xid);
Index: src/backend/utils/init/globals.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
--- a/src/backend/utils/init/globals.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/utils/init/globals.c	(date 1660678064048)
@@ -150,3 +150,5 @@
 
 int			VacuumCostBalance = 0;	/* working state for vacuum */
 bool		VacuumCostActive = false;
+
+int			slru_buffers_size_scale = 2;	/* power 2 scale for SLRU buffers */
Index: src/backend/access/transam/slru.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
--- a/src/backend/access/transam/slru.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/access/transam/slru.c	(date 1660678063980)
@@ -59,6 +59,7 @@
 #include "pgstat.h"
 #include "storage/fd.h"
 #include "storage/shmem.h"
+#include "port/pg_bitutils.h"
 
 #define SlruFileName(ctl, path, seg) \
 	snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir, seg)
@@ -71,6 +72,17 @@
  */
 #define MAX_WRITEALL_BUFFERS	16
 
+/*
+ * To avoid overflowing internal arithmetic and the size_t data type, the
+ * number of buffers should not exceed this number.
+ */
+#define SLRU_MAX_ALLOWED_BUFFERS ((1024 * 1024 * 1024) / BLCKSZ)
+
+/*
+ * SLRU bank size for slotno hash banks
+ */
+#define SLRU_BANK_SIZE 8
+
 typedef struct SlruWriteAllData
 {
 	int			num_files;		/* # files actually open */
@@ -134,7 +146,7 @@
 static SlruErrorCause slru_errcause;
 static int	slru_errno;
 
-
+static void SlruAdjustNSlots(int *nslots, int *bankmask);
 static void SimpleLruZeroLSNs(SlruCtl ctl, int slotno);
 static void SimpleLruWaitIO(SlruCtl ctl, int slotno);
 static void SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata);
@@ -148,6 +160,25 @@
 									  int segpage, void *data);
 static void SlruInternalDeleteSegment(SlruCtl ctl, int segno);
 
+/*
+ * Pick number of slots and bank size optimal for hashed associative SLRU buffers.
+ * We declare SLRU nslots is always power of 2.
+ * We split SLRU to 8-sized hash banks, after some performance benchmarks.
+ * We hash pageno to banks by pageno masked by 3 upper bits.
+ */
+static void
+SlruAdjustNSlots(int *nslots, int *bankmask)
+{
+	Assert(*nslots > 0);
+	Assert(*nslots <= SLRU_MAX_ALLOWED_BUFFERS);
+
+	*nslots = (int) pg_nextpower2_32(Max(SLRU_BANK_SIZE, Min(*nslots, NBuffers / 256)));
+
+	*bankmask = *nslots / SLRU_BANK_SIZE - 1;
+
+	elog(DEBUG5, "nslots %d banksize %d nbanks %d bankmask %x", *nslots, SLRU_BANK_SIZE, *nslots / SLRU_BANK_SIZE, *bankmask);
+}
+
 /*
  * Initialization of shared memory
  */
@@ -156,6 +187,9 @@
 SimpleLruShmemSize(int nslots, int nlsns)
 {
 	Size		sz;
+	int			bankmask_ignore;
+
+	SlruAdjustNSlots(&nslots, &bankmask_ignore);
 
 	/* we assume nslots isn't so large as to risk overflow */
 	sz = MAXALIGN(sizeof(SlruSharedData));
@@ -190,6 +224,9 @@
 {
 	SlruShared	shared;
 	bool		found;
+	int			bankmask;
+
+	SlruAdjustNSlots(&nslots, &bankmask);
 
 	shared = (SlruShared) ShmemInitStruct(name,
 										  SimpleLruShmemSize(nslots, nlsns),
@@ -257,7 +294,10 @@
 		Assert(ptr - (char *) shared <= SimpleLruShmemSize(nslots, nlsns));
 	}
 	else
+	{
 		Assert(found);
+		Assert(shared->num_slots == nslots);
+	}
 
 	/*
 	 * Initialize the unshared control struct, including directory path. We
@@ -265,6 +305,7 @@
 	 */
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
+	ctl->bank_mask = bankmask;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
 
@@ -496,12 +537,14 @@
 {
 	SlruShared	shared = ctl->shared;
 	int			slotno;
+	int			bankstart = (pageno & ctl->bank_mask) * SLRU_BANK_SIZE;
+	int			bankend = bankstart + SLRU_BANK_SIZE;
 
 	/* Try to find the page while holding only shared lock */
 	LWLockAcquire(shared->ControlLock, LW_SHARED);
 
 	/* See if page is already in a buffer */
-	for (slotno = 0; slotno < shared->num_slots; slotno++)
+	for (slotno = bankstart; slotno < bankend; slotno++)
 	{
 		if (shared->page_number[slotno] == pageno &&
 			shared->page_status[slotno] != SLRU_PAGE_EMPTY &&
@@ -1030,7 +1073,10 @@
 		int			best_invalid_page_number = 0;	/* keep compiler quiet */
 
 		/* See if page already has a buffer assigned */
-		for (slotno = 0; slotno < shared->num_slots; slotno++)
+		int			bankstart = (pageno & ctl->bank_mask) * SLRU_BANK_SIZE;
+		int			bankend = bankstart + SLRU_BANK_SIZE;
+
+		for (slotno = bankstart; slotno < bankend; slotno++)
 		{
 			if (shared->page_number[slotno] == pageno &&
 				shared->page_status[slotno] != SLRU_PAGE_EMPTY)
@@ -1065,7 +1111,7 @@
 		 * multiple pages with the same lru_count.
 		 */
 		cur_count = (shared->cur_lru_count)++;
-		for (slotno = 0; slotno < shared->num_slots; slotno++)
+		for (slotno = bankstart; slotno < bankend; slotno++)
 		{
 			int			this_delta;
 			int			this_page_number;
Index: src/backend/access/transam/clog.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
--- a/src/backend/access/transam/clog.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/access/transam/clog.c	(date 1660677867877)
@@ -74,6 +74,8 @@
 #define GetLSNIndex(slotno, xid)	((slotno) * CLOG_LSNS_PER_PAGE + \
 	((xid) % (TransactionId) CLOG_XACTS_PER_PAGE) / CLOG_XACTS_PER_LSN_GROUP)
 
+#define NUM_CLOG_BUFFERS 	(128 << slru_buffers_size_scale)
+
 /*
  * The number of subtransactions below which we consider to apply clog group
  * update optimization.  Testing reveals that the number higher than this can
@@ -661,42 +663,20 @@
 	return status;
 }
 
-/*
- * Number of shared CLOG buffers.
- *
- * On larger multi-processor systems, it is possible to have many CLOG page
- * requests in flight at one time which could lead to disk access for CLOG
- * page if the required page is not found in memory.  Testing revealed that we
- * can get the best performance by having 128 CLOG buffers, more than that it
- * doesn't improve performance.
- *
- * Unconditionally keeping the number of CLOG buffers to 128 did not seem like
- * a good idea, because it would increase the minimum amount of shared memory
- * required to start, which could be a problem for people running very small
- * configurations.  The following formula seems to represent a reasonable
- * compromise: people with very low values for shared_buffers will get fewer
- * CLOG buffers as well, and everyone else will get 128.
- */
-Size
-CLOGShmemBuffers(void)
-{
-	return Min(128, Max(4, NBuffers / 512));
-}
-
 /*
  * Initialization of shared memory for CLOG
  */
 Size
 CLOGShmemSize(void)
 {
-	return SimpleLruShmemSize(CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE);
+	return SimpleLruShmemSize(NUM_CLOG_BUFFERS, CLOG_LSNS_PER_PAGE);
 }
 
 void
 CLOGShmemInit(void)
 {
 	XactCtl->PagePrecedes = CLOGPagePrecedes;
-	SimpleLruInit(XactCtl, "Xact", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
+	SimpleLruInit(XactCtl, "Xact", NUM_CLOG_BUFFERS, CLOG_LSNS_PER_PAGE,
 				  XactSLRULock, "pg_xact", LWTRANCHE_XACT_BUFFER,
 				  SYNC_HANDLER_CLOG);
 	SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE);
Index: src/include/storage/predicate.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/storage/predicate.h b/src/include/storage/predicate.h
--- a/src/include/storage/predicate.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/storage/predicate.h	(date 1660678108718)
@@ -28,7 +28,7 @@
 
 
 /* Number of SLRU buffers to use for Serial SLRU */
-#define NUM_SERIAL_BUFFERS		16
+#define NUM_SERIAL_BUFFERS	(16 << slru_buffers_size_scale)
 
 /*
  * A handle used for sharing SERIALIZABLEXACT objects between the participants
Index: src/backend/utils/misc/postgresql.conf.sample
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
--- a/src/backend/utils/misc/postgresql.conf.sample	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/utils/misc/postgresql.conf.sample	(date 1660678108650)
@@ -155,6 +155,8 @@
 					#   mmap
 					# (change requires restart)
 #min_dynamic_shared_memory = 0MB	# (change requires restart)
+#slru_buffers_size_scale = 2 # SLRU buffers size scale of power 2, range 0..7
+					# (change requires restart)
 
 # - Disk -
 
Index: src/include/access/multixact.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h
--- a/src/include/access/multixact.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/access/multixact.h	(date 1660678108678)
@@ -30,8 +30,8 @@
 #define MaxMultiXactOffset	((MultiXactOffset) 0xFFFFFFFF)
 
 /* Number of SLRU buffers to use for multixact */
-#define NUM_MULTIXACTOFFSET_BUFFERS		8
-#define NUM_MULTIXACTMEMBER_BUFFERS		16
+#define NUM_MULTIXACTOFFSET_BUFFERS		(16 << slru_buffers_size_scale)
+#define NUM_MULTIXACTMEMBER_BUFFERS		(32 << slru_buffers_size_scale)
 
 /*
  * Possible multixact lock modes ("status").  The first four modes are for
Index: src/backend/utils/misc/guc.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
--- a/src/backend/utils/misc/guc.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/utils/misc/guc.c	(date 1660678064216)
@@ -32,9 +32,11 @@
 #endif
 #include <unistd.h>
 
+#include "access/clog.h"
 #include "access/commit_ts.h"
 #include "access/gin.h"
 #include "access/rmgr.h"
+#include "access/slru.h"
 #include "access/tableam.h"
 #include "access/toast_compression.h"
 #include "access/transam.h"
@@ -2375,6 +2377,16 @@
 		-1, -1, INT_MAX,
 		NULL, NULL, NULL
 	},
+
+	{
+		{"slru_buffers_size_scale", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("SLRU buffers size scale of power 2"),
+			NULL
+		},
+		&slru_buffers_size_scale,
+		2, 0, 7,
+		NULL, NULL, NULL
+	},
 
 	{
 		{"temp_buffers", PGC_USERSET, RESOURCES_MEM,
Index: src/include/commands/async.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/commands/async.h b/src/include/commands/async.h
--- a/src/include/commands/async.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/commands/async.h	(date 1660678108702)
@@ -18,7 +18,7 @@
 /*
  * The number of SLRU page buffers we use for the notification queue.
  */
-#define NUM_NOTIFY_BUFFERS	8
+#define NUM_NOTIFY_BUFFERS	(16 << slru_buffers_size_scale)
 
 extern bool Trace_notify;
 extern volatile sig_atomic_t notifyInterruptPending;
Index: src/include/access/slru.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
--- a/src/include/access/slru.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/access/slru.h	(date 1660678108690)
@@ -134,6 +134,11 @@
 	 * it's always the same, it doesn't need to be in shared memory.
 	 */
 	char		Dir[64];
+
+	/*
+	 * mask for slotno hash bank
+	 */
+	Size		bank_mask;
 } SlruCtlData;
 
 typedef SlruCtlData *SlruCtl;
Index: src/backend/access/transam/subtrans.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
--- a/src/backend/access/transam/subtrans.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/access/transam/subtrans.c	(date 1660678064032)
@@ -31,6 +31,7 @@
 #include "access/slru.h"
 #include "access/subtrans.h"
 #include "access/transam.h"
+#include "miscadmin.h"
 #include "pg_trace.h"
 #include "utils/snapmgr.h"
 
Index: doc/src/sgml/config.sgml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
--- a/doc/src/sgml/config.sgml	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/doc/src/sgml/config.sgml	(date 1660677867861)
@@ -1953,6 +1953,37 @@
       </listitem>
      </varlistentry>
 
+    <varlistentry id="guc-slru-buffers-size-scale" xreflabel="slru_buffers_size_scale">
+     <term><varname>slru_buffers_size_scale</varname> (<type>integer</type>)
+     <indexterm>
+      <primary><varname>slru_buffers_size_scale</varname> configuration parameter</primary>
+     </indexterm>
+     </term>
+     <listitem>
+      <para>
+       Specifies power 2 scale for all SLRU shared memory buffers sizes. Buffers sizes depends on
+       both <literal>guc_slru_buffers_size_scale</literal> and <literal>shared_buffers</literal> params.
+      </para>
+      <para>
+       This affects on buffers in the list below (see also <xref linkend="pgdata-contents-table"/>):
+        <itemizedlist>
+         <listitem><para><literal>NUM_MULTIXACTOFFSET_BUFFERS = Min(32 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_MULTIXACTMEMBER_BUFFERS = Min(64 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_SUBTRANS_BUFFERS = Min(64 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_NOTIFY_BUFFERS = Min(32 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_SERIAL_BUFFERS = Min(32 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_CLOG_BUFFERS = Min(128 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_COMMIT_TS_BUFFERS = Min(128 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+        </itemizedlist>
+      </para>
+      <para>
+       Value is in <literal>0..7</literal> bounds.
+       The default value is <literal>2</literal>.
+       This parameter can only be set at server start.
+      </para>
+     </listitem>
+    </varlistentry>
+
      <varlistentry id="guc-max-stack-depth" xreflabel="max_stack_depth">
       <term><varname>max_stack_depth</varname> (<type>integer</type>)
       <indexterm>
Index: src/backend/access/transam/commit_ts.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
--- a/src/backend/access/transam/commit_ts.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/access/transam/commit_ts.c	(date 1660677867889)
@@ -73,6 +73,8 @@
 #define TransactionIdToCTsEntry(xid)	\
 	((xid) % (TransactionId) COMMIT_TS_XACTS_PER_PAGE)
 
+#define NUM_COMMIT_TS_BUFFERS	(128 << slru_buffers_size_scale)
+
 /*
  * Link to shared-memory data structures for CommitTs control
  */
@@ -508,26 +510,13 @@
 	PG_RETURN_DATUM(HeapTupleGetDatum(htup));
 }
 
-/*
- * Number of shared CommitTS buffers.
- *
- * We use a very similar logic as for the number of CLOG buffers (except we
- * scale up twice as fast with shared buffers, and the maximum is twice as
- * high); see comments in CLOGShmemBuffers.
- */
-Size
-CommitTsShmemBuffers(void)
-{
-	return Min(256, Max(4, NBuffers / 256));
-}
-
 /*
  * Shared memory sizing for CommitTs
  */
 Size
 CommitTsShmemSize(void)
 {
-	return SimpleLruShmemSize(CommitTsShmemBuffers(), 0) +
+	return SimpleLruShmemSize(NUM_COMMIT_TS_BUFFERS, 0) +
 		sizeof(CommitTimestampShared);
 }
 
@@ -541,7 +530,7 @@
 	bool		found;
 
 	CommitTsCtl->PagePrecedes = CommitTsPagePrecedes;
-	SimpleLruInit(CommitTsCtl, "CommitTs", CommitTsShmemBuffers(), 0,
+	SimpleLruInit(CommitTsCtl, "CommitTs", NUM_COMMIT_TS_BUFFERS, 0,
 				  CommitTsSLRULock, "pg_commit_ts",
 				  LWTRANCHE_COMMITTS_BUFFER,
 				  SYNC_HANDLER_COMMIT_TS);


^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2022-08-18 03:35  Andrey Borodin <[email protected]>
  parent: [email protected]
  0 siblings, 1 reply; 18+ messages in thread

From: Andrey Borodin @ 2022-08-18 03:35 UTC (permalink / raw)
  To: [email protected]; +Cc: Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers



> On 17 Aug 2022, at 00:36, [email protected] wrote:
> 
>> Andrey Borodin wrote 2022-07-23 11:39:
>> Yura, thank you for your benchmarks!
>> We already knew that patch can save the day on pathological workloads,
>> now we have a proof of this.
>> Also there's the evidence that user can blindly increase size of SLRU
>> if they want (with the second patch). So there's no need for hard
>> explanations on how to tune the buffers size.
> 
> Hi @Andrey.Borodin, With some considerations and performance checks from @Yura.Sokolov we simplified your approach by the following:
> 
> 1. Preamble. We feel free to increase any SLRU's, since there's no performance degradation on large Buffers count using your SLRU buckets solution.
> 2. `slru_buffers_size_scale` is only one config param introduced for all SLRUs. It scales SLRUs upper cap by power 2.
> 3. All SLRU buffers count are capped by both `MBuffers (shared_buffers)` and `slru_buffers_size_scale`. see
> 4. Magic initial constants `NUM_*_BUFFERS << slru_buffers_size_scale` are applied for every SLRU.
> 5. All SLRU buffers are always sized as power of 2, their hash bucket size is always 8.
> 
> There's attached patch for your consideration. It does gather and simplify both `v21-0001-Make-all-SLRU-buffer-sizes-configurable.patch` and `v21-0002-Divide-SLRU-buffers-into-8-associative-banks.patch` to much simpler approach.

I like the idea of one knob instead of one per each SLRU. Maybe we even could deduce sane value from NBuffers? That would effectively lead to 0 knobs :)

Your patch have a prefix "v22-0006", does it mean there are 5 previous steps of the patchset?

Thank you!


Best regards, Andrey Borodin.




^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2022-08-19 15:48  [email protected]
  parent: Andrey Borodin <[email protected]>
  0 siblings, 1 reply; 18+ messages in thread

From: [email protected] @ 2022-08-19 15:48 UTC (permalink / raw)
  To: Andrey Borodin <[email protected]>; +Cc: Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

Andrey Borodin wrote 2022-08-18 06:35:
> 
> I like the idea of one knob instead of one per each SLRU. Maybe we
> even could deduce sane value from NBuffers? That would effectively
> lead to 0 knobs :)
> 
> Your patch have a prefix "v22-0006", does it mean there are 5 previous
> steps of the patchset?
> 
> Thank you!
> 
> 
> Best regards, Andrey Borodin.

Not sure it's possible to deduce from NBuffers only. 
slru_buffers_scale_shift looks like relief valve for systems with ultra 
scaled NBuffers.

Regarding v22-0006 I just tried to choose index unique for this thread 
so now it's fixed to 0001 indexing.

Attachments:

  [text/x-diff] v23-0001-bucketed-SLRUs-simplified.patch (18.0K, ../../[email protected]/2-v23-0001-bucketed-SLRUs-simplified.patch)
  download | inline diff:
Index: src/include/miscadmin.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
--- a/src/include/miscadmin.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/miscadmin.h	(date 1660678108730)
@@ -176,6 +176,7 @@
 extern PGDLLIMPORT int MaxConnections;
 extern PGDLLIMPORT int max_worker_processes;
 extern PGDLLIMPORT int max_parallel_workers;
+extern PGDLLIMPORT int slru_buffers_size_scale;
 
 extern PGDLLIMPORT int MyProcPid;
 extern PGDLLIMPORT pg_time_t MyStartTime;
Index: src/include/access/subtrans.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/access/subtrans.h b/src/include/access/subtrans.h
--- a/src/include/access/subtrans.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/access/subtrans.h	(date 1660678108698)
@@ -12,7 +12,7 @@
 #define SUBTRANS_H
 
 /* Number of SLRU buffers to use for subtrans */
-#define NUM_SUBTRANS_BUFFERS	32
+#define NUM_SUBTRANS_BUFFERS	(32 << slru_buffers_size_scale)
 
 extern void SubTransSetParent(TransactionId xid, TransactionId parent);
 extern TransactionId SubTransGetParent(TransactionId xid);
Index: src/backend/utils/init/globals.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
--- a/src/backend/utils/init/globals.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/utils/init/globals.c	(date 1660678064048)
@@ -150,3 +150,5 @@
 
 int			VacuumCostBalance = 0;	/* working state for vacuum */
 bool		VacuumCostActive = false;
+
+int			slru_buffers_size_scale = 2;	/* power 2 scale for SLRU buffers */
Index: src/backend/access/transam/slru.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
--- a/src/backend/access/transam/slru.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/access/transam/slru.c	(date 1660678063980)
@@ -59,6 +59,7 @@
 #include "pgstat.h"
 #include "storage/fd.h"
 #include "storage/shmem.h"
+#include "port/pg_bitutils.h"
 
 #define SlruFileName(ctl, path, seg) \
 	snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir, seg)
@@ -71,6 +72,17 @@
  */
 #define MAX_WRITEALL_BUFFERS	16
 
+/*
+ * To avoid overflowing internal arithmetic and the size_t data type, the
+ * number of buffers should not exceed this number.
+ */
+#define SLRU_MAX_ALLOWED_BUFFERS ((1024 * 1024 * 1024) / BLCKSZ)
+
+/*
+ * SLRU bank size for slotno hash banks
+ */
+#define SLRU_BANK_SIZE 8
+
 typedef struct SlruWriteAllData
 {
 	int			num_files;		/* # files actually open */
@@ -134,7 +146,7 @@
 static SlruErrorCause slru_errcause;
 static int	slru_errno;
 
-
+static void SlruAdjustNSlots(int *nslots, int *bankmask);
 static void SimpleLruZeroLSNs(SlruCtl ctl, int slotno);
 static void SimpleLruWaitIO(SlruCtl ctl, int slotno);
 static void SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata);
@@ -148,6 +160,25 @@
 									  int segpage, void *data);
 static void SlruInternalDeleteSegment(SlruCtl ctl, int segno);
 
+/*
+ * Pick number of slots and bank size optimal for hashed associative SLRU buffers.
+ * We declare SLRU nslots is always power of 2.
+ * We split SLRU to 8-sized hash banks, after some performance benchmarks.
+ * We hash pageno to banks by pageno masked by 3 upper bits.
+ */
+static void
+SlruAdjustNSlots(int *nslots, int *bankmask)
+{
+	Assert(*nslots > 0);
+	Assert(*nslots <= SLRU_MAX_ALLOWED_BUFFERS);
+
+	*nslots = (int) pg_nextpower2_32(Max(SLRU_BANK_SIZE, Min(*nslots, NBuffers / 256)));
+
+	*bankmask = *nslots / SLRU_BANK_SIZE - 1;
+
+	elog(DEBUG5, "nslots %d banksize %d nbanks %d bankmask %x", *nslots, SLRU_BANK_SIZE, *nslots / SLRU_BANK_SIZE, *bankmask);
+}
+
 /*
  * Initialization of shared memory
  */
@@ -156,6 +187,9 @@
 SimpleLruShmemSize(int nslots, int nlsns)
 {
 	Size		sz;
+	int			bankmask_ignore;
+
+	SlruAdjustNSlots(&nslots, &bankmask_ignore);
 
 	/* we assume nslots isn't so large as to risk overflow */
 	sz = MAXALIGN(sizeof(SlruSharedData));
@@ -190,6 +224,9 @@
 {
 	SlruShared	shared;
 	bool		found;
+	int			bankmask;
+
+	SlruAdjustNSlots(&nslots, &bankmask);
 
 	shared = (SlruShared) ShmemInitStruct(name,
 										  SimpleLruShmemSize(nslots, nlsns),
@@ -257,7 +294,10 @@
 		Assert(ptr - (char *) shared <= SimpleLruShmemSize(nslots, nlsns));
 	}
 	else
+	{
 		Assert(found);
+		Assert(shared->num_slots == nslots);
+	}
 
 	/*
 	 * Initialize the unshared control struct, including directory path. We
@@ -265,6 +305,7 @@
 	 */
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
+	ctl->bank_mask = bankmask;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
 
@@ -496,12 +537,14 @@
 {
 	SlruShared	shared = ctl->shared;
 	int			slotno;
+	int			bankstart = (pageno & ctl->bank_mask) * SLRU_BANK_SIZE;
+	int			bankend = bankstart + SLRU_BANK_SIZE;
 
 	/* Try to find the page while holding only shared lock */
 	LWLockAcquire(shared->ControlLock, LW_SHARED);
 
 	/* See if page is already in a buffer */
-	for (slotno = 0; slotno < shared->num_slots; slotno++)
+	for (slotno = bankstart; slotno < bankend; slotno++)
 	{
 		if (shared->page_number[slotno] == pageno &&
 			shared->page_status[slotno] != SLRU_PAGE_EMPTY &&
@@ -1030,7 +1073,10 @@
 		int			best_invalid_page_number = 0;	/* keep compiler quiet */
 
 		/* See if page already has a buffer assigned */
-		for (slotno = 0; slotno < shared->num_slots; slotno++)
+		int			bankstart = (pageno & ctl->bank_mask) * SLRU_BANK_SIZE;
+		int			bankend = bankstart + SLRU_BANK_SIZE;
+
+		for (slotno = bankstart; slotno < bankend; slotno++)
 		{
 			if (shared->page_number[slotno] == pageno &&
 				shared->page_status[slotno] != SLRU_PAGE_EMPTY)
@@ -1065,7 +1111,7 @@
 		 * multiple pages with the same lru_count.
 		 */
 		cur_count = (shared->cur_lru_count)++;
-		for (slotno = 0; slotno < shared->num_slots; slotno++)
+		for (slotno = bankstart; slotno < bankend; slotno++)
 		{
 			int			this_delta;
 			int			this_page_number;
Index: src/backend/access/transam/clog.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
--- a/src/backend/access/transam/clog.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/access/transam/clog.c	(date 1660677867877)
@@ -74,6 +74,8 @@
 #define GetLSNIndex(slotno, xid)	((slotno) * CLOG_LSNS_PER_PAGE + \
 	((xid) % (TransactionId) CLOG_XACTS_PER_PAGE) / CLOG_XACTS_PER_LSN_GROUP)
 
+#define NUM_CLOG_BUFFERS 	(128 << slru_buffers_size_scale)
+
 /*
  * The number of subtransactions below which we consider to apply clog group
  * update optimization.  Testing reveals that the number higher than this can
@@ -661,42 +663,20 @@
 	return status;
 }
 
-/*
- * Number of shared CLOG buffers.
- *
- * On larger multi-processor systems, it is possible to have many CLOG page
- * requests in flight at one time which could lead to disk access for CLOG
- * page if the required page is not found in memory.  Testing revealed that we
- * can get the best performance by having 128 CLOG buffers, more than that it
- * doesn't improve performance.
- *
- * Unconditionally keeping the number of CLOG buffers to 128 did not seem like
- * a good idea, because it would increase the minimum amount of shared memory
- * required to start, which could be a problem for people running very small
- * configurations.  The following formula seems to represent a reasonable
- * compromise: people with very low values for shared_buffers will get fewer
- * CLOG buffers as well, and everyone else will get 128.
- */
-Size
-CLOGShmemBuffers(void)
-{
-	return Min(128, Max(4, NBuffers / 512));
-}
-
 /*
  * Initialization of shared memory for CLOG
  */
 Size
 CLOGShmemSize(void)
 {
-	return SimpleLruShmemSize(CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE);
+	return SimpleLruShmemSize(NUM_CLOG_BUFFERS, CLOG_LSNS_PER_PAGE);
 }
 
 void
 CLOGShmemInit(void)
 {
 	XactCtl->PagePrecedes = CLOGPagePrecedes;
-	SimpleLruInit(XactCtl, "Xact", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
+	SimpleLruInit(XactCtl, "Xact", NUM_CLOG_BUFFERS, CLOG_LSNS_PER_PAGE,
 				  XactSLRULock, "pg_xact", LWTRANCHE_XACT_BUFFER,
 				  SYNC_HANDLER_CLOG);
 	SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE);
Index: src/include/storage/predicate.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/storage/predicate.h b/src/include/storage/predicate.h
--- a/src/include/storage/predicate.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/storage/predicate.h	(date 1660678108718)
@@ -28,7 +28,7 @@
 
 
 /* Number of SLRU buffers to use for Serial SLRU */
-#define NUM_SERIAL_BUFFERS		16
+#define NUM_SERIAL_BUFFERS	(16 << slru_buffers_size_scale)
 
 /*
  * A handle used for sharing SERIALIZABLEXACT objects between the participants
Index: src/backend/utils/misc/postgresql.conf.sample
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
--- a/src/backend/utils/misc/postgresql.conf.sample	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/utils/misc/postgresql.conf.sample	(date 1660678108650)
@@ -155,6 +155,8 @@
 					#   mmap
 					# (change requires restart)
 #min_dynamic_shared_memory = 0MB	# (change requires restart)
+#slru_buffers_size_scale = 2 # SLRU buffers size scale of power 2, range 0..7
+					# (change requires restart)
 
 # - Disk -
 
Index: src/include/access/multixact.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h
--- a/src/include/access/multixact.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/access/multixact.h	(date 1660678108678)
@@ -30,8 +30,8 @@
 #define MaxMultiXactOffset	((MultiXactOffset) 0xFFFFFFFF)
 
 /* Number of SLRU buffers to use for multixact */
-#define NUM_MULTIXACTOFFSET_BUFFERS		8
-#define NUM_MULTIXACTMEMBER_BUFFERS		16
+#define NUM_MULTIXACTOFFSET_BUFFERS		(16 << slru_buffers_size_scale)
+#define NUM_MULTIXACTMEMBER_BUFFERS		(32 << slru_buffers_size_scale)
 
 /*
  * Possible multixact lock modes ("status").  The first four modes are for
Index: src/backend/utils/misc/guc.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
--- a/src/backend/utils/misc/guc.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/utils/misc/guc.c	(date 1660678064216)
@@ -32,9 +32,11 @@
 #endif
 #include <unistd.h>
 
+#include "access/clog.h"
 #include "access/commit_ts.h"
 #include "access/gin.h"
 #include "access/rmgr.h"
+#include "access/slru.h"
 #include "access/tableam.h"
 #include "access/toast_compression.h"
 #include "access/transam.h"
@@ -2375,6 +2377,16 @@
 		-1, -1, INT_MAX,
 		NULL, NULL, NULL
 	},
+
+	{
+		{"slru_buffers_size_scale", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("SLRU buffers size scale of power 2"),
+			NULL
+		},
+		&slru_buffers_size_scale,
+		2, 0, 7,
+		NULL, NULL, NULL
+	},
 
 	{
 		{"temp_buffers", PGC_USERSET, RESOURCES_MEM,
Index: src/include/commands/async.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/commands/async.h b/src/include/commands/async.h
--- a/src/include/commands/async.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/commands/async.h	(date 1660678108702)
@@ -18,7 +18,7 @@
 /*
  * The number of SLRU page buffers we use for the notification queue.
  */
-#define NUM_NOTIFY_BUFFERS	8
+#define NUM_NOTIFY_BUFFERS	(16 << slru_buffers_size_scale)
 
 extern bool Trace_notify;
 extern volatile sig_atomic_t notifyInterruptPending;
Index: src/include/access/slru.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
--- a/src/include/access/slru.h	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/include/access/slru.h	(date 1660678108690)
@@ -134,6 +134,11 @@
 	 * it's always the same, it doesn't need to be in shared memory.
 	 */
 	char		Dir[64];
+
+	/*
+	 * mask for slotno hash bank
+	 */
+	Size		bank_mask;
 } SlruCtlData;
 
 typedef SlruCtlData *SlruCtl;
Index: src/backend/access/transam/subtrans.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
--- a/src/backend/access/transam/subtrans.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/access/transam/subtrans.c	(date 1660678064032)
@@ -31,6 +31,7 @@
 #include "access/slru.h"
 #include "access/subtrans.h"
 #include "access/transam.h"
+#include "miscadmin.h"
 #include "pg_trace.h"
 #include "utils/snapmgr.h"
 
Index: doc/src/sgml/config.sgml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
--- a/doc/src/sgml/config.sgml	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/doc/src/sgml/config.sgml	(date 1660677867861)
@@ -1953,6 +1953,37 @@
       </listitem>
      </varlistentry>
 
+    <varlistentry id="guc-slru-buffers-size-scale" xreflabel="slru_buffers_size_scale">
+     <term><varname>slru_buffers_size_scale</varname> (<type>integer</type>)
+     <indexterm>
+      <primary><varname>slru_buffers_size_scale</varname> configuration parameter</primary>
+     </indexterm>
+     </term>
+     <listitem>
+      <para>
+       Specifies power 2 scale for all SLRU shared memory buffers sizes. Buffers sizes depends on
+       both <literal>guc_slru_buffers_size_scale</literal> and <literal>shared_buffers</literal> params.
+      </para>
+      <para>
+       This affects on buffers in the list below (see also <xref linkend="pgdata-contents-table"/>):
+        <itemizedlist>
+         <listitem><para><literal>NUM_MULTIXACTOFFSET_BUFFERS = Min(32 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_MULTIXACTMEMBER_BUFFERS = Min(64 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_SUBTRANS_BUFFERS = Min(64 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_NOTIFY_BUFFERS = Min(32 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_SERIAL_BUFFERS = Min(32 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_CLOG_BUFFERS = Min(128 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_COMMIT_TS_BUFFERS = Min(128 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+        </itemizedlist>
+      </para>
+      <para>
+       Value is in <literal>0..7</literal> bounds.
+       The default value is <literal>2</literal>.
+       This parameter can only be set at server start.
+      </para>
+     </listitem>
+    </varlistentry>
+
      <varlistentry id="guc-max-stack-depth" xreflabel="max_stack_depth">
       <term><varname>max_stack_depth</varname> (<type>integer</type>)
       <indexterm>
Index: src/backend/access/transam/commit_ts.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
--- a/src/backend/access/transam/commit_ts.c	(revision 020258fbd30d37ddd03d0ec68264d1544f8d2838)
+++ b/src/backend/access/transam/commit_ts.c	(date 1660677867889)
@@ -73,6 +73,8 @@
 #define TransactionIdToCTsEntry(xid)	\
 	((xid) % (TransactionId) COMMIT_TS_XACTS_PER_PAGE)
 
+#define NUM_COMMIT_TS_BUFFERS	(128 << slru_buffers_size_scale)
+
 /*
  * Link to shared-memory data structures for CommitTs control
  */
@@ -508,26 +510,13 @@
 	PG_RETURN_DATUM(HeapTupleGetDatum(htup));
 }
 
-/*
- * Number of shared CommitTS buffers.
- *
- * We use a very similar logic as for the number of CLOG buffers (except we
- * scale up twice as fast with shared buffers, and the maximum is twice as
- * high); see comments in CLOGShmemBuffers.
- */
-Size
-CommitTsShmemBuffers(void)
-{
-	return Min(256, Max(4, NBuffers / 256));
-}
-
 /*
  * Shared memory sizing for CommitTs
  */
 Size
 CommitTsShmemSize(void)
 {
-	return SimpleLruShmemSize(CommitTsShmemBuffers(), 0) +
+	return SimpleLruShmemSize(NUM_COMMIT_TS_BUFFERS, 0) +
 		sizeof(CommitTimestampShared);
 }
 
@@ -541,7 +530,7 @@
 	bool		found;
 
 	CommitTsCtl->PagePrecedes = CommitTsPagePrecedes;
-	SimpleLruInit(CommitTsCtl, "CommitTs", CommitTsShmemBuffers(), 0,
+	SimpleLruInit(CommitTsCtl, "CommitTs", NUM_COMMIT_TS_BUFFERS, 0,
 				  CommitTsSLRULock, "pg_commit_ts",
 				  LWTRANCHE_COMMITTS_BUFFER,
 				  SYNC_HANDLER_COMMIT_TS);


^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2022-12-20 18:39  Andrey Borodin <[email protected]>
  parent: Thomas Munro <[email protected]>
  0 siblings, 0 replies; 18+ messages in thread

From: Andrey Borodin @ 2022-12-20 18:39 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Andrey Borodin <[email protected]>; Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

On Sat, Jul 23, 2022 at 1:48 AM Thomas Munro <[email protected]> wrote:
>
> On Sat, Jul 23, 2022 at 8:41 PM Andrey Borodin <[email protected]> wrote:
> > Thomas, do you still have any doubts? Or is it certain that SLRU will be replaced by any better subsystem in 16?
>
> Hi Andrey,
>
> Sorry for my lack of replies on this and the other SLRU thread -- I'm
> thinking and experimenting.  More soon.
>

Hi Thomas,

PostgreSQL 16 feature freeze is approaching again. Let's choose
something from possible solutions, even if the chosen one is
temporary.
Thank you!

Best regards, Andrey Borodin.





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2023-01-03 13:01  vignesh C <[email protected]>
  parent: [email protected]
  0 siblings, 1 reply; 18+ messages in thread

From: vignesh C @ 2023-01-03 13:01 UTC (permalink / raw)
  To: [email protected]; +Cc: Andrey Borodin <[email protected]>; Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

On Fri, 19 Aug 2022 at 21:18, <[email protected]> wrote:
>
> Andrey Borodin wrote 2022-08-18 06:35:
> >
> > I like the idea of one knob instead of one per each SLRU. Maybe we
> > even could deduce sane value from NBuffers? That would effectively
> > lead to 0 knobs :)
> >
> > Your patch have a prefix "v22-0006", does it mean there are 5 previous
> > steps of the patchset?
> >
> > Thank you!
> >
> >
> > Best regards, Andrey Borodin.
>
> Not sure it's possible to deduce from NBuffers only.
> slru_buffers_scale_shift looks like relief valve for systems with ultra
> scaled NBuffers.
>
> Regarding v22-0006 I just tried to choose index unique for this thread
> so now it's fixed to 0001 indexing.

The patch does not apply on top of HEAD as in [1], please post a rebased patch:

=== Applying patches on top of PostgreSQL commit ID
325bc54eed4ea0836a0bb715bb18342f0c1c668a ===
=== applying patch ./v23-0001-bucketed-SLRUs-simplified.patch
patching file src/include/miscadmin.h
...
patching file src/backend/utils/misc/guc.c
Hunk #1 FAILED at 32.
Hunk #2 FAILED at 2375.
2 out of 2 hunks FAILED -- saving rejects to file
src/backend/utils/misc/guc.c.rej

[1] - http://cfbot.cputube.org/patch_41_2627.log

Regards,
Vignesh






^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2023-01-09 04:19  Andrey Borodin <[email protected]>
  parent: vignesh C <[email protected]>
  0 siblings, 2 replies; 18+ messages in thread

From: Andrey Borodin @ 2023-01-09 04:19 UTC (permalink / raw)
  To: vignesh C <[email protected]>; +Cc: [email protected]; Andrey Borodin <[email protected]>; Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

On Tue, Jan 3, 2023 at 5:02 AM vignesh C <[email protected]> wrote:
> does not apply on top of HEAD as in [1], please post a rebased patch:
>
Thanks! Here's the rebase.

Best regards, Andrey Borodin.


Attachments:

  [application/octet-stream] v24-0001-Divide-SLRU-buffers-into-8-associative-banks.patch (17.1K, ../../CAAhFRxheL9XKBiyPYtTj9p1pupkorXFdSM4OjNGB79xn6o=x4Q@mail.gmail.com/2-v24-0001-Divide-SLRU-buffers-into-8-associative-banks.patch)
  download | inline diff:
From e0a990996b2b5fe78747b82718b4806038d439c8 Mon Sep 17 00:00:00 2001
From: "Andrey M. Borodin" <[email protected]>
Date: Sun, 8 Jan 2023 20:15:15 -0800
Subject: [PATCH v24] Divide SLRU buffers into 8-associative banks

We want to eliminate linear search within SLRU buffers.
To do so we divide SLRU buffers into banks. Each bank holds
approximately 8 buffers. Each SLRU pageno may reside only in one bank.
Adjacent pagenos reside in different banks.

Also invent slru_buffers_size_scale to control SLRU buffers.
---
 doc/src/sgml/config.sgml                      | 31 +++++++++++
 src/backend/access/transam/clog.c             | 28 ++--------
 src/backend/access/transam/commit_ts.c        | 19 ++-----
 src/backend/access/transam/slru.c             | 54 +++++++++++++++++--
 src/backend/access/transam/subtrans.c         |  1 +
 src/backend/utils/init/globals.c              |  2 +
 src/backend/utils/misc/guc_tables.c           | 10 ++++
 src/backend/utils/misc/postgresql.conf.sample |  2 +
 src/include/access/multixact.h                |  4 +-
 src/include/access/slru.h                     |  5 ++
 src/include/access/subtrans.h                 |  2 +-
 src/include/commands/async.h                  |  2 +-
 src/include/miscadmin.h                       |  2 +
 src/include/storage/predicate.h               |  2 +-
 14 files changed, 116 insertions(+), 48 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 05b3862d09..4ebc4bc17b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1968,6 +1968,37 @@ include_dir 'conf.d'
       </listitem>
      </varlistentry>
 
+    <varlistentry id="guc-slru-buffers-size-scale" xreflabel="slru_buffers_size_scale">
+     <term><varname>slru_buffers_size_scale</varname> (<type>integer</type>)
+     <indexterm>
+      <primary><varname>slru_buffers_size_scale</varname> configuration parameter</primary>
+     </indexterm>
+     </term>
+     <listitem>
+      <para>
+       Specifies power 2 scale for all SLRU shared memory buffers sizes. Buffers sizes depends on
+       both <literal>guc_slru_buffers_size_scale</literal> and <literal>shared_buffers</literal> params.
+      </para>
+      <para>
+       This affects on buffers in the list below (see also <xref linkend="pgdata-contents-table"/>):
+        <itemizedlist>
+         <listitem><para><literal>NUM_MULTIXACTOFFSET_BUFFERS = Min(32 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_MULTIXACTMEMBER_BUFFERS = Min(64 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_SUBTRANS_BUFFERS = Min(64 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_NOTIFY_BUFFERS = Min(32 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_SERIAL_BUFFERS = Min(32 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_CLOG_BUFFERS = Min(128 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+         <listitem><para><literal>NUM_COMMIT_TS_BUFFERS = Min(128 &lt;&lt; slru_buffers_size_scale, shared_buffers/256)</literal></para></listitem>
+        </itemizedlist>
+      </para>
+      <para>
+       Value is in <literal>0..7</literal> bounds.
+       The default value is <literal>2</literal>.
+       This parameter can only be set at server start.
+      </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/clog.c b/src/backend/access/transam/clog.c
index 4a431d5876..29d58f1eb3 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -74,6 +74,8 @@
 #define GetLSNIndex(slotno, xid)	((slotno) * CLOG_LSNS_PER_PAGE + \
 	((xid) % (TransactionId) CLOG_XACTS_PER_PAGE) / CLOG_XACTS_PER_LSN_GROUP)
 
+#define NUM_CLOG_BUFFERS 	(128 << slru_buffers_size_scale)
+
 /*
  * The number of subtransactions below which we consider to apply clog group
  * update optimization.  Testing reveals that the number higher than this can
@@ -660,42 +662,20 @@ TransactionIdGetStatus(TransactionId xid, XLogRecPtr *lsn)
 	return status;
 }
 
-/*
- * Number of shared CLOG buffers.
- *
- * On larger multi-processor systems, it is possible to have many CLOG page
- * requests in flight at one time which could lead to disk access for CLOG
- * page if the required page is not found in memory.  Testing revealed that we
- * can get the best performance by having 128 CLOG buffers, more than that it
- * doesn't improve performance.
- *
- * Unconditionally keeping the number of CLOG buffers to 128 did not seem like
- * a good idea, because it would increase the minimum amount of shared memory
- * required to start, which could be a problem for people running very small
- * configurations.  The following formula seems to represent a reasonable
- * compromise: people with very low values for shared_buffers will get fewer
- * CLOG buffers as well, and everyone else will get 128.
- */
-Size
-CLOGShmemBuffers(void)
-{
-	return Min(128, Max(4, NBuffers / 512));
-}
-
 /*
  * Initialization of shared memory for CLOG
  */
 Size
 CLOGShmemSize(void)
 {
-	return SimpleLruShmemSize(CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE);
+	return SimpleLruShmemSize(NUM_CLOG_BUFFERS, CLOG_LSNS_PER_PAGE);
 }
 
 void
 CLOGShmemInit(void)
 {
 	XactCtl->PagePrecedes = CLOGPagePrecedes;
-	SimpleLruInit(XactCtl, "Xact", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
+	SimpleLruInit(XactCtl, "Xact", NUM_CLOG_BUFFERS, CLOG_LSNS_PER_PAGE,
 				  XactSLRULock, "pg_xact", LWTRANCHE_XACT_BUFFER,
 				  SYNC_HANDLER_CLOG);
 	SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE);
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index b897fabc70..54422f2780 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -70,6 +70,8 @@ typedef struct CommitTimestampEntry
 #define TransactionIdToCTsEntry(xid)	\
 	((xid) % (TransactionId) COMMIT_TS_XACTS_PER_PAGE)
 
+#define NUM_COMMIT_TS_BUFFERS	(128 << slru_buffers_size_scale)
+
 /*
  * Link to shared-memory data structures for CommitTs control
  */
@@ -487,26 +489,13 @@ pg_xact_commit_timestamp_origin(PG_FUNCTION_ARGS)
 	PG_RETURN_DATUM(HeapTupleGetDatum(htup));
 }
 
-/*
- * Number of shared CommitTS buffers.
- *
- * We use a very similar logic as for the number of CLOG buffers (except we
- * scale up twice as fast with shared buffers, and the maximum is twice as
- * high); see comments in CLOGShmemBuffers.
- */
-Size
-CommitTsShmemBuffers(void)
-{
-	return Min(256, Max(4, NBuffers / 256));
-}
-
 /*
  * Shared memory sizing for CommitTs
  */
 Size
 CommitTsShmemSize(void)
 {
-	return SimpleLruShmemSize(CommitTsShmemBuffers(), 0) +
+	return SimpleLruShmemSize(NUM_COMMIT_TS_BUFFERS, 0) +
 		sizeof(CommitTimestampShared);
 }
 
@@ -520,7 +509,7 @@ CommitTsShmemInit(void)
 	bool		found;
 
 	CommitTsCtl->PagePrecedes = CommitTsPagePrecedes;
-	SimpleLruInit(CommitTsCtl, "CommitTs", CommitTsShmemBuffers(), 0,
+	SimpleLruInit(CommitTsCtl, "CommitTs", NUM_COMMIT_TS_BUFFERS, 0,
 				  CommitTsSLRULock, "pg_commit_ts",
 				  LWTRANCHE_COMMITTS_BUFFER,
 				  SYNC_HANDLER_COMMIT_TS);
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 5ab86238a9..67dd967236 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -59,6 +59,7 @@
 #include "pgstat.h"
 #include "storage/fd.h"
 #include "storage/shmem.h"
+#include "port/pg_bitutils.h"
 
 #define SlruFileName(ctl, path, seg) \
 	snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir, seg)
@@ -71,6 +72,17 @@
  */
 #define MAX_WRITEALL_BUFFERS	16
 
+/*
+ * To avoid overflowing internal arithmetic and the size_t data type, the
+ * number of buffers should not exceed this number.
+ */
+#define SLRU_MAX_ALLOWED_BUFFERS ((1024 * 1024 * 1024) / BLCKSZ)
+
+/*
+ * SLRU bank size for slotno hash banks
+ */
+#define SLRU_BANK_SIZE 8
+
 typedef struct SlruWriteAllData
 {
 	int			num_files;		/* # files actually open */
@@ -134,7 +146,7 @@ typedef enum
 static SlruErrorCause slru_errcause;
 static int	slru_errno;
 
-
+static void SlruAdjustNSlots(int *nslots, int *bankmask);
 static void SimpleLruZeroLSNs(SlruCtl ctl, int slotno);
 static void SimpleLruWaitIO(SlruCtl ctl, int slotno);
 static void SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata);
@@ -148,6 +160,25 @@ static bool SlruScanDirCbDeleteCutoff(SlruCtl ctl, char *filename,
 									  int segpage, void *data);
 static void SlruInternalDeleteSegment(SlruCtl ctl, int segno);
 
+/*
+ * Pick number of slots and bank size optimal for hashed associative SLRU buffers.
+ * We declare SLRU nslots is always power of 2.
+ * We split SLRU to 8-sized hash banks, after some performance benchmarks.
+ * We hash pageno to banks by pageno masked by 3 upper bits.
+ */
+static void
+SlruAdjustNSlots(int *nslots, int *bankmask)
+{
+	Assert(*nslots > 0);
+	Assert(*nslots <= SLRU_MAX_ALLOWED_BUFFERS);
+
+	*nslots = (int) pg_nextpower2_32(Max(SLRU_BANK_SIZE, Min(*nslots, NBuffers / 256)));
+
+	*bankmask = *nslots / SLRU_BANK_SIZE - 1;
+
+	elog(DEBUG5, "nslots %d banksize %d nbanks %d bankmask %x", *nslots, SLRU_BANK_SIZE, *nslots / SLRU_BANK_SIZE, *bankmask);
+}
+
 /*
  * Initialization of shared memory
  */
@@ -156,6 +187,9 @@ Size
 SimpleLruShmemSize(int nslots, int nlsns)
 {
 	Size		sz;
+	int			bankmask_ignore;
+
+	SlruAdjustNSlots(&nslots, &bankmask_ignore);
 
 	/* we assume nslots isn't so large as to risk overflow */
 	sz = MAXALIGN(sizeof(SlruSharedData));
@@ -191,6 +225,9 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 {
 	SlruShared	shared;
 	bool		found;
+	int			bankmask;
+
+	SlruAdjustNSlots(&nslots, &bankmask);
 
 	shared = (SlruShared) ShmemInitStruct(name,
 										  SimpleLruShmemSize(nslots, nlsns),
@@ -258,7 +295,10 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 		Assert(ptr - (char *) shared <= SimpleLruShmemSize(nslots, nlsns));
 	}
 	else
+	{
 		Assert(found);
+		Assert(shared->num_slots == nslots);
+	}
 
 	/*
 	 * Initialize the unshared control struct, including directory path. We
@@ -266,6 +306,7 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 	 */
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
+	ctl->bank_mask = bankmask;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
 
@@ -497,12 +538,14 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid)
 {
 	SlruShared	shared = ctl->shared;
 	int			slotno;
+	int			bankstart = (pageno & ctl->bank_mask) * SLRU_BANK_SIZE;
+	int			bankend = bankstart + SLRU_BANK_SIZE;
 
 	/* Try to find the page while holding only shared lock */
 	LWLockAcquire(shared->ControlLock, LW_SHARED);
 
 	/* See if page is already in a buffer */
-	for (slotno = 0; slotno < shared->num_slots; slotno++)
+	for (slotno = bankstart; slotno < bankend; slotno++)
 	{
 		if (shared->page_number[slotno] == pageno &&
 			shared->page_status[slotno] != SLRU_PAGE_EMPTY &&
@@ -1031,7 +1074,10 @@ SlruSelectLRUPage(SlruCtl ctl, int pageno)
 		int			best_invalid_page_number = 0;	/* keep compiler quiet */
 
 		/* See if page already has a buffer assigned */
-		for (slotno = 0; slotno < shared->num_slots; slotno++)
+		int			bankstart = (pageno & ctl->bank_mask) * SLRU_BANK_SIZE;
+		int			bankend = bankstart + SLRU_BANK_SIZE;
+
+		for (slotno = bankstart; slotno < bankend; slotno++)
 		{
 			if (shared->page_number[slotno] == pageno &&
 				shared->page_status[slotno] != SLRU_PAGE_EMPTY)
@@ -1066,7 +1112,7 @@ SlruSelectLRUPage(SlruCtl ctl, int pageno)
 		 * multiple pages with the same lru_count.
 		 */
 		cur_count = (shared->cur_lru_count)++;
-		for (slotno = 0; slotno < shared->num_slots; slotno++)
+		for (slotno = bankstart; slotno < bankend; slotno++)
 		{
 			int			this_delta;
 			int			this_page_number;
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index 62bb610167..125273e235 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -31,6 +31,7 @@
 #include "access/slru.h"
 #include "access/subtrans.h"
 #include "access/transam.h"
+#include "miscadmin.h"
 #include "pg_trace.h"
 #include "utils/snapmgr.h"
 
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 1b1d814254..65a509ac5a 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -151,3 +151,5 @@ int64		VacuumPageDirty = 0;
 
 int			VacuumCostBalance = 0;	/* working state for vacuum */
 bool		VacuumCostActive = false;
+
+int			slru_buffers_size_scale = 2;	/* power 2 scale for SLRU buffers */
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 68328b1402..3abc06e7e1 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2216,6 +2216,16 @@ struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"slru_buffers_size_scale", PGC_POSTMASTER, RESOURCES_MEM,
+			gettext_noop("SLRU buffers size scale of power 2"),
+			NULL
+		},
+		&slru_buffers_size_scale,
+		2, 0, 7,
+		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/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 5afdeb04de..69ba328ea4 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -155,6 +155,8 @@
 					#   mmap
 					# (change requires restart)
 #min_dynamic_shared_memory = 0MB	# (change requires restart)
+#slru_buffers_size_scale = 2 # SLRU buffers size scale of power 2, range 0..7
+					# (change requires restart)
 
 # - Disk -
 
diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h
index 246f757f6a..6a2c914d48 100644
--- a/src/include/access/multixact.h
+++ b/src/include/access/multixact.h
@@ -30,8 +30,8 @@
 #define MaxMultiXactOffset	((MultiXactOffset) 0xFFFFFFFF)
 
 /* Number of SLRU buffers to use for multixact */
-#define NUM_MULTIXACTOFFSET_BUFFERS		8
-#define NUM_MULTIXACTMEMBER_BUFFERS		16
+#define NUM_MULTIXACTOFFSET_BUFFERS		(16 << slru_buffers_size_scale)
+#define NUM_MULTIXACTMEMBER_BUFFERS		(32 << slru_buffers_size_scale)
 
 /*
  * Possible multixact lock modes ("status").  The first four modes are for
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index a8a424d92d..f5f2b5b8b5 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -134,6 +134,11 @@ typedef struct SlruCtlData
 	 * it's always the same, it doesn't need to be in shared memory.
 	 */
 	char		Dir[64];
+
+	/*
+	 * mask for slotno hash bank
+	 */
+	Size		bank_mask;
 } SlruCtlData;
 
 typedef SlruCtlData *SlruCtl;
diff --git a/src/include/access/subtrans.h b/src/include/access/subtrans.h
index 46a473c77f..0dad287550 100644
--- a/src/include/access/subtrans.h
+++ b/src/include/access/subtrans.h
@@ -12,7 +12,7 @@
 #define SUBTRANS_H
 
 /* Number of SLRU buffers to use for subtrans */
-#define NUM_SUBTRANS_BUFFERS	32
+#define NUM_SUBTRANS_BUFFERS	(32 << slru_buffers_size_scale)
 
 extern void SubTransSetParent(TransactionId xid, TransactionId parent);
 extern TransactionId SubTransGetParent(TransactionId xid);
diff --git a/src/include/commands/async.h b/src/include/commands/async.h
index 02da6ba7e1..b1d59472b1 100644
--- a/src/include/commands/async.h
+++ b/src/include/commands/async.h
@@ -18,7 +18,7 @@
 /*
  * The number of SLRU page buffers we use for the notification queue.
  */
-#define NUM_NOTIFY_BUFFERS	8
+#define NUM_NOTIFY_BUFFERS	(16 << slru_buffers_size_scale)
 
 extern PGDLLIMPORT bool Trace_notify;
 extern PGDLLIMPORT volatile sig_atomic_t notifyInterruptPending;
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 0ffeefc437..789f00a7ea 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -177,6 +177,7 @@ extern PGDLLIMPORT int MaxBackends;
 extern PGDLLIMPORT int MaxConnections;
 extern PGDLLIMPORT int max_worker_processes;
 extern PGDLLIMPORT int max_parallel_workers;
+extern PGDLLIMPORT int slru_buffers_size_scale;
 
 extern PGDLLIMPORT int MyProcPid;
 extern PGDLLIMPORT pg_time_t MyStartTime;
@@ -262,6 +263,7 @@ extern PGDLLIMPORT int work_mem;
 extern PGDLLIMPORT double hash_mem_multiplier;
 extern PGDLLIMPORT int maintenance_work_mem;
 extern PGDLLIMPORT int max_parallel_maintenance_workers;
+extern PGDLLIMPORT int slru_buffers_size_scale;
 
 extern PGDLLIMPORT int VacuumCostPageHit;
 extern PGDLLIMPORT int VacuumCostPageMiss;
diff --git a/src/include/storage/predicate.h b/src/include/storage/predicate.h
index cd48afa17b..794ecd8169 100644
--- a/src/include/storage/predicate.h
+++ b/src/include/storage/predicate.h
@@ -28,7 +28,7 @@ extern PGDLLIMPORT int max_predicate_locks_per_page;
 
 
 /* Number of SLRU buffers to use for Serial SLRU */
-#define NUM_SERIAL_BUFFERS		16
+#define NUM_SERIAL_BUFFERS	(16 << slru_buffers_size_scale)
 
 /*
  * A handle used for sharing SERIALIZABLEXACT objects between the participants
-- 
2.32.0 (Apple Git-132)



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2023-01-11 05:58  Dilip Kumar <[email protected]>
  parent: Andrey Borodin <[email protected]>
  1 sibling, 1 reply; 18+ messages in thread

From: Dilip Kumar @ 2023-01-11 05:58 UTC (permalink / raw)
  To: Andrey Borodin <[email protected]>; +Cc: vignesh C <[email protected]>; [email protected]; Andrey Borodin <[email protected]>; Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

On Mon, Jan 9, 2023 at 9:49 AM Andrey Borodin <[email protected]> wrote:
>
> On Tue, Jan 3, 2023 at 5:02 AM vignesh C <[email protected]> wrote:
> > does not apply on top of HEAD as in [1], please post a rebased patch:
> >
> Thanks! Here's the rebase.

I was looking into this patch, it seems like three different
optimizations are squeezed in a single patch
1) dividing buffer space in banks to reduce the seq search cost 2) guc
parameter for buffer size scale 3) some of the buffer size values are
modified compared to what it is on the head.  I think these are 3
patches which should be independently committable.

While looking into the first idea of dividing the buffer space in
banks, I see that it will speed up finding the buffers but OTOH while
searching the victim buffer it will actually can hurt the performance
the slru pages which are frequently accessed are not evenly
distributed across the banks.  So imagine the cases where we have some
banks with a lot of empty slots and other banks from which we
frequently have to evict out the pages in order to get the new pages
in.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2023-01-11 17:13  Andrey Borodin <[email protected]>
  parent: Dilip Kumar <[email protected]>
  0 siblings, 0 replies; 18+ messages in thread

From: Andrey Borodin @ 2023-01-11 17:13 UTC (permalink / raw)
  To: Dilip Kumar <[email protected]>; +Cc: vignesh C <[email protected]>; [email protected]; Andrey Borodin <[email protected]>; Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

Hi Dilip! Thank you for the review!

On Tue, Jan 10, 2023 at 9:58 PM Dilip Kumar <[email protected]> wrote:
>
> On Mon, Jan 9, 2023 at 9:49 AM Andrey Borodin <[email protected]> wrote:
> >
> > On Tue, Jan 3, 2023 at 5:02 AM vignesh C <[email protected]> wrote:
> > > does not apply on top of HEAD as in [1], please post a rebased patch:
> > >
> > Thanks! Here's the rebase.
>
> I was looking into this patch, it seems like three different
> optimizations are squeezed in a single patch
> 1) dividing buffer space in banks to reduce the seq search cost 2) guc
> parameter for buffer size scale 3) some of the buffer size values are
> modified compared to what it is on the head.  I think these are 3
> patches which should be independently committable.
There's no point in dividing SLRU buffers in parts unless the buffer's
size is configurable.
And it's only possible to enlarge default buffers size if SLRU buffers
are divided into banks.
So the features can be viewed as independent commits, but make no
sense separately.

But, probably, it's a good idea to split the patch back anyway, for
easier review.

>
> While looking into the first idea of dividing the buffer space in
> banks, I see that it will speed up finding the buffers but OTOH while
> searching the victim buffer it will actually can hurt the performance
> the slru pages which are frequently accessed are not evenly
> distributed across the banks.  So imagine the cases where we have some
> banks with a lot of empty slots and other banks from which we
> frequently have to evict out the pages in order to get the new pages
> in.
>

Yes. Despite the extremely low probability of such a case, this
pattern when a user accesses pages assigned to only one bank may
happen.
This case is equivalent to having just one bank, which means small
buffers. Just as we have now.

Thank you!

Best regards, Andrey Borodin.






^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* [PATCH v2 1/4] misc binaryheap fixes
@ 2023-07-19 22:54  Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 18+ messages in thread

From: Nathan Bossart @ 2023-07-19 22:54 UTC (permalink / raw)

---
 src/backend/postmaster/pgarch.c                 | 12 ++++++------
 src/backend/replication/logical/reorderbuffer.c |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 46af349564..c28e6f2070 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -108,7 +108,7 @@ static ArchiveModuleState *archive_module_state;
  * archive_status.  Minimizing the number of directory scans when there are
  * many files to archive can significantly improve archival rate.
  *
- * arch_heap is a max-heap that is used during the directory scan to track
+ * arch_heap is a min-heap that is used during the directory scan to track
  * the highest-priority files to archive.  After the directory scan
  * completes, the file names are stored in ascending order of priority in
  * arch_files.  pgarch_readyXlog() returns files from arch_files until it
@@ -248,7 +248,7 @@ PgArchiverMain(void)
 	arch_files = palloc(sizeof(struct arch_files_state));
 	arch_files->arch_files_size = 0;
 
-	/* Initialize our max-heap for prioritizing files to archive. */
+	/* Initialize our min-heap for prioritizing files to archive. */
 	arch_files->arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN,
 												ready_file_comparator, NULL);
 
@@ -624,7 +624,7 @@ pgarch_readyXlog(char *xlog)
 		basename[basenamelen] = '\0';
 
 		/*
-		 * Store the file in our max-heap if it has a high enough priority.
+		 * Store the file in our min-heap if it has a high enough priority.
 		 */
 		if (arch_files->arch_heap->bh_size < NUM_FILES_PER_DIRECTORY_SCAN)
 		{
@@ -644,15 +644,15 @@ pgarch_readyXlog(char *xlog)
 			 * Remove the lowest priority file and add the current one to the
 			 * heap.
 			 */
-			arch_file = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap));
+			arch_file = DatumGetCString(binaryheap_first(arch_files->arch_heap));
 			strcpy(arch_file, basename);
-			binaryheap_add(arch_files->arch_heap, CStringGetDatum(arch_file));
+			binaryheap_replace_first(arch_files->arch_heap, CStringGetDatum(arch_file));
 		}
 	}
 	FreeDir(rldir);
 
 	/* If no files were found, simply return. */
-	if (arch_files->arch_heap->bh_size == 0)
+	if (binaryheap_empty(arch_files->arch_heap))
 		return false;
 
 	/*
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 26d252bd87..05bbcecd29 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1381,7 +1381,7 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state)
 	int32		off;
 
 	/* nothing there anymore */
-	if (state->heap->bh_size == 0)
+	if (binaryheap_empty(state->heap))
 		return NULL;
 
 	off = DatumGetInt32(binaryheap_first(state->heap));
-- 
2.25.1


--qDbXVdCdHGoSgWSk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0002-make-binaryheap-available-to-frontend.patch"



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2024-01-20 03:31  vignesh C <[email protected]>
  parent: Andrey Borodin <[email protected]>
  1 sibling, 1 reply; 18+ messages in thread

From: vignesh C @ 2024-01-20 03:31 UTC (permalink / raw)
  To: Andrey Borodin <[email protected]>; +Cc: [email protected]; Andrey Borodin <[email protected]>; Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

On Mon, 9 Jan 2023 at 09:49, Andrey Borodin <[email protected]> wrote:
>
> On Tue, Jan 3, 2023 at 5:02 AM vignesh C <[email protected]> wrote:
> > does not apply on top of HEAD as in [1], please post a rebased patch:
> >
> Thanks! Here's the rebase.

I'm seeing that there has been no activity in this thread for more
than 1 year now, I'm planning to close this in the current commitfest
unless someone is planning to take it forward.

Regards,
Vignesh





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2024-01-27 04:58  Andrey Borodin <[email protected]>
  parent: vignesh C <[email protected]>
  0 siblings, 1 reply; 18+ messages in thread

From: Andrey Borodin @ 2024-01-27 04:58 UTC (permalink / raw)
  To: vignesh C <[email protected]>; +Cc: Andrew Borodin <[email protected]>; [email protected]; Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers



> On 20 Jan 2024, at 08:31, vignesh C <[email protected]> wrote:
> 
> On Mon, 9 Jan 2023 at 09:49, Andrey Borodin <[email protected]> wrote:
>> 
>> On Tue, Jan 3, 2023 at 5:02 AM vignesh C <[email protected]> wrote:
>>> does not apply on top of HEAD as in [1], please post a rebased patch:
>>> 
>> Thanks! Here's the rebase.
> 
> I'm seeing that there has been no activity in this thread for more
> than 1 year now, I'm planning to close this in the current commitfest
> unless someone is planning to take it forward.

Hi Vignesh,

thanks for the ping! Most important parts of this patch set are discussed in [0]. If that patchset will be committed, I'll withdraw entry for this thread from commitfest.
There's a version of Multixact-specific optimizations [1], but I hope they will not be necessary with effective caches developed in [0]. It seems to me that most important part of those optimization is removing sleeps under SLRU lock on standby [2] by Kyotaro Horiguchi. But given that cache optimizations took 4 years to get closer to commit, I'm not sure we will get this optimization any time soon...

Thanks!

Best regards, Andrey Borodin.


[0] https://www.postgresql.org/message-id/flat/CAFiTN-vzDvNz%3DExGXz6gdyjtzGixKSqs0mKHMmaQ8sOSEFZ33A%40m...
[1] https://www.postgresql.org/message-id/flat/2ECE132B-C042-4489-930E-DBC5D0DAB84A%40yandex-team.ru#5f7...
[2] https://www.postgresql.org/message-id/flat/20200515.090333.24867479329066911.horikyota.ntt%40gmail.c...




^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2024-01-28 12:49  Alvaro Herrera <[email protected]>
  parent: Andrey Borodin <[email protected]>
  0 siblings, 1 reply; 18+ messages in thread

From: Alvaro Herrera @ 2024-01-28 12:49 UTC (permalink / raw)
  To: Andrey Borodin <[email protected]>; +Cc: vignesh C <[email protected]>; Andrew Borodin <[email protected]>; [email protected]; Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers

On 2024-Jan-27, Andrey Borodin wrote:

> thanks for the ping! Most important parts of this patch set are discussed in [0]. If that patchset will be committed, I'll withdraw entry for this thread from commitfest.
> There's a version of Multixact-specific optimizations [1], but I hope they will not be necessary with effective caches developed in [0]. It seems to me that most important part of those optimization is removing sleeps under SLRU lock on standby [2] by Kyotaro Horiguchi. But given that cache optimizations took 4 years to get closer to commit, I'm not sure we will get this optimization any time soon...

I'd appreciate it if you or Horiguchi-san can update his patch to remove
use of usleep in favor of a CV in multixact, and keep this CF entry to
cover it.

Perhaps a test to make the code reach the usleep(1000) can be written
using injection points (49cd2b93d7db)?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"La experiencia nos dice que el hombre peló millones de veces las patatas,
pero era forzoso admitir la posibilidad de que en un caso entre millones,
las patatas pelarían al hombre" (Ijon Tichy)





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: MultiXact\SLRU buffers configuration
@ 2024-01-28 18:17  Andrey M. Borodin <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 18+ messages in thread

From: Andrey M. Borodin @ 2024-01-28 18:17 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: vignesh C <[email protected]>; Andrew Borodin <[email protected]>; [email protected]; Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; Thomas Munro <[email protected]>; Gilles Darold <[email protected]>; Tomas Vondra <[email protected]>; Tomas Vondra <[email protected]>; Alexander Korotkov <[email protected]>; Anastasia Lubennikova <[email protected]>; Daniel Gustafsson <[email protected]>; Kyotaro Horiguchi <[email protected]>; pgsql-hackers



> On 28 Jan 2024, at 17:49, Alvaro Herrera <[email protected]> wrote:
> 
> I'd appreciate it if you or Horiguchi-san can update his patch to remove
> use of usleep in favor of a CV in multixact, and keep this CF entry to
> cover it.

Sure! Sounds great!

> Perhaps a test to make the code reach the usleep(1000) can be written
> using injection points (49cd2b93d7db)?

I've tried to prototype something like that. But interesting point between GetNewMultiXactId() and RecordNewMultiXact() is a critical section, and we cannot have injection points in critical sections...
Also, to implement such a test we need "wait" type of injection points, see step 2 in attachment. With this type of injection points I can stop a backend amidst entering information about new MultiXact.


Best regards, Andrey Borodin.



Attachments:

  [application/octet-stream] 0001-Add-conditional-variable-to-wait-for-next-MultXact-o.patch (3.3K, ../../[email protected]/2-0001-Add-conditional-variable-to-wait-for-next-MultXact-o.patch)
  download | inline diff:
From 975eb3448acbec97c14d48f21261e44aca7b3acc Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Fri, 22 May 2020 14:13:33 +0500
Subject: [PATCH 1/3] Add conditional variable to wait for next MultXact offset
 in edge case

---
 src/backend/access/transam/multixact.c        | 23 ++++++++++++++++++-
 .../utils/activity/wait_event_names.txt       |  1 +
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 59523be901..03fcd25d4c 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
@@ -894,6 +896,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;
@@ -1388,9 +1398,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_NEXT_MXMEMBERS);
+			ConditionVariableCancelSleep();
 			goto retry;
 		}
 
@@ -1875,6 +1895,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/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index a5df835dd4..1bb78d5aad 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -144,6 +144,7 @@ SYNC_REP	"Waiting for confirmation from a remote server during synchronous repli
 WAL_RECEIVER_EXIT	"Waiting for the WAL receiver to exit."
 WAL_RECEIVER_WAIT_START	"Waiting for startup process to send initial data for streaming replication."
 WAL_SUMMARY_READY	"Waiting for a new WAL summary to be generated."
+NEXT_MXMEMBERS	"Waiting for a next multixact member to be filled."
 XACT_GROUP_UPDATE	"Waiting for the group leader to update transaction status at end of a parallel operation."
 
 
-- 
2.37.1 (Apple Git-137.1)



  [application/octet-stream] 0002-Add-wait-type-for-injection-points.patch (3.7K, ../../[email protected]/3-0002-Add-wait-type-for-injection-points.patch)
  download | inline diff:
From 93b4c7dac290d6d2d212522b1a10fef03372315e Mon Sep 17 00:00:00 2001
From: "Andrey M. Borodin" <[email protected]>
Date: Sun, 28 Jan 2024 22:22:22 +0500
Subject: [PATCH 2/3] Add wait type for injection points

---
 src/backend/utils/misc/injection_point.c      | 20 +++++++++++++++++++
 src/include/utils/injection_point.h           |  1 +
 src/test/modules/injection_points/Makefile    |  1 +
 .../injection_points/injection_points.c       | 16 +++++++++++++++
 4 files changed, 38 insertions(+)

diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 0cf4d51cac..398ef2cf30 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -252,6 +252,26 @@ InjectionPointDetach(const char *name)
 #endif
 }
 
+/*
+ * Test if injection point is attached.
+ */
+bool
+InjectionPointIsAttach(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+	bool		found;
+
+	LWLockAcquire(InjectionPointLock, LW_EXCLUSIVE);
+	hash_search(InjectionPointHash, name, HASH_FIND, &found);
+	LWLockRelease(InjectionPointLock);
+
+	return found;
+
+#else
+	elog(ERROR, "Injection points are not supported by this build");
+#endif
+}
+
 /*
  * Execute an injection point, if defined.
  *
diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index 55524b568f..e07f6b7024 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -33,5 +33,6 @@ extern void InjectionPointAttach(const char *name,
 								 const char *function);
 extern void InjectionPointRun(const char *name);
 extern void InjectionPointDetach(const char *name);
+extern bool InjectionPointIsAttach(const char *name);
 
 #endif							/* INJECTION_POINT_H */
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 2cbbae4e0a..543d2ab927 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -7,6 +7,7 @@ DATA = injection_points--1.0.sql
 PGFILEDESC = "injection_points - facility for injection points"
 
 REGRESS = injection_points
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index e843e6594f..fbb30b15ad 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -18,6 +18,7 @@
 #include "postgres.h"
 
 #include "fmgr.h"
+#include "miscadmin.h"
 #include "storage/lwlock.h"
 #include "storage/shmem.h"
 #include "utils/builtins.h"
@@ -28,6 +29,7 @@ PG_MODULE_MAGIC;
 
 extern PGDLLEXPORT void injection_error(const char *name);
 extern PGDLLEXPORT void injection_notice(const char *name);
+extern PGDLLEXPORT void injection_wait(const char *name);
 
 
 /* Set of callbacks available to be attached to an injection point. */
@@ -43,6 +45,18 @@ injection_notice(const char *name)
 	elog(NOTICE, "notice triggered for injection point %s", name);
 }
 
+void
+injection_wait(const char *name)
+{
+	elog(NOTICE, "waiting triggered for injection point %s", name);
+	do
+	{
+		CHECK_FOR_INTERRUPTS();
+		pg_usleep(1000L);
+	} while (InjectionPointIsAttach(name));
+	elog(NOTICE, "waiting done for injection point %s", name);
+}
+
 /*
  * SQL function for creating an injection point.
  */
@@ -58,6 +72,8 @@ injection_points_attach(PG_FUNCTION_ARGS)
 		function = "injection_error";
 	else if (strcmp(action, "notice") == 0)
 		function = "injection_notice";
+	else if (strcmp(action, "wait") == 0)
+		function = "injection_wait";
 	else
 		elog(ERROR, "incorrect action \"%s\" for injection point creation", action);
 
-- 
2.37.1 (Apple Git-137.1)



  [application/octet-stream] 0003-Try-to-test-multixact-CV-sleep.patch (5.3K, ../../[email protected]/4-0003-Try-to-test-multixact-CV-sleep.patch)
  download | inline diff:
From a61958407770b6cb864e0b245a4341e033824f46 Mon Sep 17 00:00:00 2001
From: "Andrey M. Borodin" <[email protected]>
Date: Sun, 28 Jan 2024 23:10:55 +0500
Subject: [PATCH 3/3] Try to test multixact CV sleep

---
 src/backend/access/transam/multixact.c        |  5 ++
 .../injection_points--1.0.sql                 | 11 ++++
 .../injection_points/injection_points.c       | 26 ++++++++
 .../modules/injection_points/t/001_wait.pl    | 64 +++++++++++++++++++
 4 files changed, 106 insertions(+)
 create mode 100644 src/test/modules/injection_points/t/001_wait.pl

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 03fcd25d4c..15b2a0010a 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -89,6 +89,7 @@
 #include "storage/proc.h"
 #include "storage/procarray.h"
 #include "utils/builtins.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 #include "utils/snapmgr.h"
 
@@ -1200,6 +1201,8 @@ GetNewMultiXactId(int nmembers, MultiXactOffset *offset)
 
 	LWLockRelease(MultiXactGenLock);
 
+	INJECTION_POINT("GetNewMultiXactId-done");
+
 	debug_elog4(DEBUG2, "GetNew: returning %u offset %u", result, *offset);
 	return result;
 }
@@ -1408,6 +1411,8 @@ retry:
 			LWLockRelease(MultiXactOffsetSLRULock);
 			CHECK_FOR_INTERRUPTS();
 
+			INJECTION_POINT("GetMultiXactIdMembers-CV-sleep");
+
 			ConditionVariableSleep(&MultiXactState->nextoff_cv,
 								   WAIT_EVENT_NEXT_MXMEMBERS);
 			ConditionVariableCancelSleep();
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 5944c41716..d3ebda5964 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -33,3 +33,14 @@ CREATE FUNCTION injection_points_detach(IN point_name TEXT)
 RETURNS void
 AS 'MODULE_PATHNAME', 'injection_points_detach'
 LANGUAGE C STRICT PARALLEL UNSAFE;
+
+
+CREATE FUNCTION create_test_multixact()
+RETURNS xid
+AS 'MODULE_PATHNAME', 'create_test_multixact'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
+CREATE FUNCTION read_test_multixact(xid)
+RETURNS void
+AS 'MODULE_PATHNAME', 'read_test_multixact'
+LANGUAGE C STRICT PARALLEL UNSAFE;
\ No newline at end of file
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index fbb30b15ad..d3d5faaa25 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -109,3 +109,29 @@ injection_points_detach(PG_FUNCTION_ARGS)
 
 	PG_RETURN_VOID();
 }
+
+#include "access/multixact.h"
+#include "access/xact.h"
+
+PG_FUNCTION_INFO_V1(create_test_multixact);
+Datum
+create_test_multixact(PG_FUNCTION_ARGS)
+{
+	MultiXactId id;
+	MultiXactIdSetOldestMember();
+	id = MultiXactIdCreate(GetCurrentTransactionId(), MultiXactStatusUpdate,
+						GetCurrentTransactionId(), MultiXactStatusForShare);
+	PG_RETURN_TRANSACTIONID(id);
+}
+
+PG_FUNCTION_INFO_V1(read_test_multixact);
+Datum
+read_test_multixact(PG_FUNCTION_ARGS)
+{
+	MultiXactId id = PG_GETARG_TRANSACTIONID(0);
+	MultiXactMember *members;
+	INJECTION_POINT("read_test_multixact");
+	if (GetMultiXactIdMembers(id,&members,false, false) == -1)
+		elog(ERROR, "MultiXactId not found");
+	PG_RETURN_VOID();
+}
\ No newline at end of file
diff --git a/src/test/modules/injection_points/t/001_wait.pl b/src/test/modules/injection_points/t/001_wait.pl
new file mode 100644
index 0000000000..6076ffc6ea
--- /dev/null
+++ b/src/test/modules/injection_points/t/001_wait.pl
@@ -0,0 +1,64 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+my ($node, $result);
+
+$node = PostgreSQL::Test::Cluster->new('injection_points');
+$node->init;
+$node->start;
+$node->safe_psql('postgres', q(CREATE EXTENSION injection_points));
+
+$result = $node->psql('postgres', q(select injection_points_attach('FIRST','wait')));
+is($result, '0', 'wait injection point set');
+
+my $bg = $node->background_psql('postgres');
+
+$bg->query_until(
+	qr/start/, q(
+\echo start
+select injection_points_run('FIRST');
+select injection_points_attach('SECOND','wait');
+));
+
+$result = $node->psql('postgres', q(
+select injection_points_run('SECOND');
+select injection_points_detach('FIRST');
+));
+is($result, '0', 'wait injection point set');
+
+$bg->quit;
+
+$node->safe_psql('postgres', q(select injection_points_attach('read_test_multixact','wait')));
+$node->safe_psql('postgres', q(select injection_points_attach('GetMultiXactIdMembers-CV-sleep','notice')));
+
+my $observer = $node->background_psql('postgres');
+
+$observer->query_safe(
+	q(
+select read_test_multixact(create_test_multixact());
+));
+
+$node->safe_psql('postgres', q(select injection_points_attach('GetNewMultiXactId-done','wait')));
+
+my $creator = $node->background_psql('postgres');
+
+$creator->query_safe( q(select create_test_multixact();));
+
+$node->safe_psql('postgres', q(select injection_points_detach('read_test_multixact')));
+
+$node->safe_psql('postgres', q(select injection_points_detach('GetNewMultiXactId-done')));
+
+$observer->quit;
+
+$creator->quit;
+
+$node->stop;
+done_testing();
-- 
2.37.1 (Apple Git-137.1)



^ permalink  raw  reply  [nested|flat] 18+ messages in thread


end of thread, other threads:[~2024-01-28 18:17 UTC | newest]

Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-02-19 21:42 Re: MultiXact\SLRU buffers configuration Andres Freund <[email protected]>
2022-02-20 07:35 ` Andrey Borodin <[email protected]>
2022-07-21 13:00   ` Yura Sokolov <[email protected]>
2022-07-23 08:39     ` Andrey Borodin <[email protected]>
2022-07-23 08:47       ` Thomas Munro <[email protected]>
2022-12-20 18:39         ` Andrey Borodin <[email protected]>
2022-08-16 19:36       ` [email protected]
2022-08-18 03:35         ` Andrey Borodin <[email protected]>
2022-08-19 15:48           ` [email protected]
2023-01-03 13:01             ` vignesh C <[email protected]>
2023-01-09 04:19               ` Andrey Borodin <[email protected]>
2023-01-11 05:58                 ` Dilip Kumar <[email protected]>
2023-01-11 17:13                   ` Andrey Borodin <[email protected]>
2024-01-20 03:31                 ` vignesh C <[email protected]>
2024-01-27 04:58                   ` Andrey Borodin <[email protected]>
2024-01-28 12:49                     ` Alvaro Herrera <[email protected]>
2024-01-28 18:17                       ` Andrey M. Borodin <[email protected]>
2023-07-19 22:54 [PATCH v2 1/4] misc binaryheap fixes Nathan Bossart <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox