Re: MultiXact\SLRU buffers configuration
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
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 nsl
Re: MultiXact\SLRU buffers configuration
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.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
