public inbox for [email protected]
help / color / mirror / Atom feedFrom: Zhijie Hou (Fujitsu) <[email protected]>
To: shveta malik <[email protected]>
To: Ajin Cherian <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Subject: RE: Synchronizing slots from primary to standby
Date: Wed, 13 Mar 2024 07:58:52 +0000
Message-ID: <OS0PR01MB571671A91E602BEFD3083EEE942A2@OS0PR01MB5716.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAJpy0uDkY+sw+uaRHGSORpq9GgATxEaj36=YW+rMm+b35FzFHA@mail.gmail.com>
References: <CAFPTHDaC6mQECXQUPUoMXkxPo+23Gwx7LeHvtdmuXKSWCMTQgw@mail.gmail.com>
<CAJpy0uDkY+sw+uaRHGSORpq9GgATxEaj36=YW+rMm+b35FzFHA@mail.gmail.com>
On Friday, March 8, 2024 1:09 PM shveta malik <[email protected]> wrote:
> On Fri, Mar 8, 2024 at 9:56 AM Ajin Cherian <[email protected]> wrote:
> >
> >> Pushed with minor modifications. I'll keep an eye on BF.
> >>
> >> BTW, one thing that we should try to evaluate a bit more is the
> >> traversal of slots in StandbySlotsHaveCaughtup() where we verify if
> >> all the slots mentioned in standby_slot_names have received the
> >> required WAL. Even if the standby_slot_names list is short the total
> >> number of slots can be much larger which can lead to an increase in
> >> CPU usage during traversal. There is an optimization that allows to
> >> cache ss_oldest_flush_lsn and ensures that we don't need to traverse
> >> the slots each time so it may not hit frequently but still there is a
> >> chance. I see it is possible to further optimize this area by caching
> >> the position of each slot mentioned in standby_slot_names in
> >> replication_slots array but not sure whether it is worth.
> >>
> >>
> >
> > I tried to test this by configuring a large number of logical slots while making
> sure the standby slots are at the end of the array and checking if there was any
> performance hit in logical replication from these searches.
> >
>
Thanks Nisha for conducting some additional tests and discussing with me
internally. We have collected the performance data on HEAD. Basically, we don't
see a noticeable difference in the performance data and StandbySlotsHaveCaughtup
also does not stand out in the profile.
Here are the details:
> 1) Redoing XLogSendLogical time-log related test with
> 'sync_replication_slots' enabled.
Setup:
- one primary + 3standbys + one subscriber with one active subscription
- ran 15 min pgbench for all cases
- hot_standby_feedback=ON and sync_replication_slots=TRUE
(To maximize the impact of SearchNamedReplicationSlot clear, the standby slot
is at the end of the ReplicationSlotCtl->replication_slots array in each test)
Case1 - 1 slot: 895.305565 secs
Case2 - 100 slots: 894.936039 secs
Case3 - 500 slots: 895.256412 secs
> 2) pg_recvlogical test to monitor lag in StandbySlotsHaveCaughtup() for a
> large number of slots.
We reran the XLogSendLogical() wait time analysis tests.
Setup:
- One primary node and 3 standby nodes
- Created logical slots using "test_decoding" and activated one walsender by running pg_recvlogical on one slot.
- hot_standby_feedback=ON and sync_replication_slots=TRUE
- Did one run for each case with pgbench for 15 min
(To maximize the impact of SearchNamedReplicationSlot clear, the stanbys slot
is at the end of the ReplicationSlotCtl->replication_slots array in each test)
Case1 - 1 slot: 894.83775 secs
Case2 - 100 slots: 894.449356 secs
Case3 - 500 slots: 894.98479 secs
There is no noticeable regression when the number of replication slots increases.
> 3) Profiling to see if StandbySlotsHaveCaughtup() is noticeable in the report
> when there are a large number of slots to traverse.
The setup is the same as 2). To maximize the impact of
SearchNamedReplicationSlot clear, the stanbys slot is at the end of the
ReplicationSlotCtl->replication_slots array.
The StandbySlotsHaveCaughtup is not noticeable in the profile.
0.03% 0.00% postgres postgres [.] StandbySlotsHaveCaughtup
After some investigation, it appears that the cached 'ss_oldest_flush_lsn'
plays a crucial role in optimizing this workload, effectively reducing the need
for frequent strcmp operations within the loop.
To test the impact of frequent strcmp calls, we conducted a test by removing
the 'ss_oldest_flush_lsn' check and re-evaluating the profile. This time, although the
profile indicated a small increase in the StandbySlotsHaveCaughtup metric,
it still does not raise significant concerns.
--1.47%--NeedToWaitForWal
| NeedToWaitForStandbys
| StandbySlotsHaveCaughtup
| |
| --0.96%--SearchNamedReplicationSlot
The scripts that were used to setup the test environment for all above tests are attached.
The machine configuration for above tests is as follows:
CPU : E7-4890v2(2.8Ghz/15core)×4
MEM : 768GB
HDD : 600GB×2
OS : RHEL 7.9
While no noticeable overhead was observed in the SearchNamedReplicationSlot
operation, we explored a strategy to enhance efficiency by minimizing the
search for standby slots within the loop. The idea is to cache the
position of each standby slot within ReplicationSlotCtl->replication_slots. We
will reference the slot directly through
ReplicationSlotCtl->replication_slots[index]. If the slot name matches, we will
perform other checks including the restart_lsn; otherwise,
SearchNamedReplicationSlot is invoked to update the index cache accordingly.
This optimization can reduce the cost from O(n*m) to O(n).
Note that since we didn't see the overhead in the test, I am not proposing to
push this patch now. But just share the idea and a small patch in case anyone
came across a workload where performance impact of SearchNamedReplicationSlot
becomes noticeable.
Best Regards,
Hou zj
From 3b24e04a225eb3bc82950432727662bdcb41f511 Mon Sep 17 00:00:00 2001
From: Hou Zhijie <[email protected]>
Date: Fri, 8 Mar 2024 09:09:26 +0800
Subject: [PATCH] Cache standby slot index
---
src/backend/replication/slot.c | 47 ++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 30db34ece8..8736d9a87b 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -93,10 +93,17 @@ typedef struct
/* Number of slot names in the slot_names[] */
int nslotnames;
+ /* The offset of the slot index array */
+ int slot_index_offset;
+
/*
* slot_names contains 'nslotnames' consecutive null-terminated C strings.
*/
char slot_names[FLEXIBLE_ARRAY_MEMBER];
+
+ /*
+ * After all the C strings, there is a size 'nslotnames' slot index array.
+ */
} StandbySlotNamesConfigData;
/*
@@ -156,6 +163,14 @@ static StandbySlotNamesConfigData *standby_slot_names_config;
*/
static XLogRecPtr ss_oldest_flush_lsn = InvalidXLogRecPtr;
+/*
+ * This array caches the position of each standby slot in
+ * ReplicationSlotCtl->replication_slots. The position will be updated if it
+ * becomes outdated due to a slot drop. See StandbySlotsHaveCaughtup for
+ * details.
+ */
+static int *standby_slot_index = NULL;
+
static void ReplicationSlotShmemExit(int code, Datum arg);
static void ReplicationSlotDropPtr(ReplicationSlot *slot);
@@ -2453,6 +2468,7 @@ check_standby_slot_names(char **newval, void **extra, GucSource source)
char *ptr;
List *elemlist;
int size;
+ int index_offset;
bool ok;
StandbySlotNamesConfigData *config;
@@ -2477,11 +2493,15 @@ check_standby_slot_names(char **newval, void **extra, GucSource source)
foreach_ptr(char, slot_name, elemlist)
size += strlen(slot_name) + 1;
+ index_offset = size;
+ size += list_length(elemlist) * sizeof(int);
+
/* GUC extra value must be guc_malloc'd, not palloc'd */
config = (StandbySlotNamesConfigData *) guc_malloc(LOG, size);
/* Transform the data into StandbySlotNamesConfigData */
config->nslotnames = list_length(elemlist);
+ config->slot_index_offset = index_offset;
ptr = config->slot_names;
foreach_ptr(char, slot_name, elemlist)
@@ -2490,6 +2510,9 @@ check_standby_slot_names(char **newval, void **extra, GucSource source)
ptr += strlen(slot_name) + 1;
}
+ /* Initialize the index array */
+ memset(ptr, -1, list_length(elemlist) * sizeof(int));
+
*extra = (void *) config;
pfree(rawname);
@@ -2510,6 +2533,12 @@ assign_standby_slot_names(const char *newval, void *extra)
ss_oldest_flush_lsn = InvalidXLogRecPtr;
standby_slot_names_config = (StandbySlotNamesConfigData *) extra;
+
+ if (extra)
+ {
+ char *ptr = (char *) extra + standby_slot_names_config->slot_index_offset;
+ standby_slot_index = (int *) ptr;
+ }
}
/*
@@ -2590,9 +2619,21 @@ StandbySlotsHaveCaughtup(XLogRecPtr wait_for_lsn, int elevel)
XLogRecPtr restart_lsn;
bool invalidated;
bool inactive;
- ReplicationSlot *slot;
+ ReplicationSlot *slot = NULL;
+ int slot_index = standby_slot_index[i];
- slot = SearchNamedReplicationSlot(name, false);
+ /* Get the replication slot using the standby slot index */
+ if (slot_index >= 0)
+ slot = &ReplicationSlotCtl->replication_slots[slot_index];
+
+ /*
+ * If the slot index has not been initialized, or if the slot found by
+ * the index does not match, which means the original slot was dropped,
+ * then we search for the named slot among all replication slots and
+ * update the index if the slot is found.
+ */
+ if (!slot || !slot->in_use || strcmp(name, NameStr(slot->data.name)) != 0)
+ slot = SearchNamedReplicationSlot(name, false);
if (!slot)
{
@@ -2616,6 +2657,8 @@ StandbySlotsHaveCaughtup(XLogRecPtr wait_for_lsn, int elevel)
break;
}
+ standby_slot_index[i] = ReplicationSlotIndex(slot);
+
if (SlotIsLogical(slot))
{
/*
--
2.30.0.windows.2
Attachments:
[text/plain] 0001-Cache-standby-slot-index.patch.txt (3.9K, ../OS0PR01MB571671A91E602BEFD3083EEE942A2@OS0PR01MB5716.jpnprd01.prod.outlook.com/2-0001-Cache-standby-slot-index.patch.txt)
download | inline diff:
From 3b24e04a225eb3bc82950432727662bdcb41f511 Mon Sep 17 00:00:00 2001
From: Hou Zhijie <[email protected]>
Date: Fri, 8 Mar 2024 09:09:26 +0800
Subject: [PATCH] Cache standby slot index
---
src/backend/replication/slot.c | 47 ++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 30db34ece8..8736d9a87b 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -93,10 +93,17 @@ typedef struct
/* Number of slot names in the slot_names[] */
int nslotnames;
+ /* The offset of the slot index array */
+ int slot_index_offset;
+
/*
* slot_names contains 'nslotnames' consecutive null-terminated C strings.
*/
char slot_names[FLEXIBLE_ARRAY_MEMBER];
+
+ /*
+ * After all the C strings, there is a size 'nslotnames' slot index array.
+ */
} StandbySlotNamesConfigData;
/*
@@ -156,6 +163,14 @@ static StandbySlotNamesConfigData *standby_slot_names_config;
*/
static XLogRecPtr ss_oldest_flush_lsn = InvalidXLogRecPtr;
+/*
+ * This array caches the position of each standby slot in
+ * ReplicationSlotCtl->replication_slots. The position will be updated if it
+ * becomes outdated due to a slot drop. See StandbySlotsHaveCaughtup for
+ * details.
+ */
+static int *standby_slot_index = NULL;
+
static void ReplicationSlotShmemExit(int code, Datum arg);
static void ReplicationSlotDropPtr(ReplicationSlot *slot);
@@ -2453,6 +2468,7 @@ check_standby_slot_names(char **newval, void **extra, GucSource source)
char *ptr;
List *elemlist;
int size;
+ int index_offset;
bool ok;
StandbySlotNamesConfigData *config;
@@ -2477,11 +2493,15 @@ check_standby_slot_names(char **newval, void **extra, GucSource source)
foreach_ptr(char, slot_name, elemlist)
size += strlen(slot_name) + 1;
+ index_offset = size;
+ size += list_length(elemlist) * sizeof(int);
+
/* GUC extra value must be guc_malloc'd, not palloc'd */
config = (StandbySlotNamesConfigData *) guc_malloc(LOG, size);
/* Transform the data into StandbySlotNamesConfigData */
config->nslotnames = list_length(elemlist);
+ config->slot_index_offset = index_offset;
ptr = config->slot_names;
foreach_ptr(char, slot_name, elemlist)
@@ -2490,6 +2510,9 @@ check_standby_slot_names(char **newval, void **extra, GucSource source)
ptr += strlen(slot_name) + 1;
}
+ /* Initialize the index array */
+ memset(ptr, -1, list_length(elemlist) * sizeof(int));
+
*extra = (void *) config;
pfree(rawname);
@@ -2510,6 +2533,12 @@ assign_standby_slot_names(const char *newval, void *extra)
ss_oldest_flush_lsn = InvalidXLogRecPtr;
standby_slot_names_config = (StandbySlotNamesConfigData *) extra;
+
+ if (extra)
+ {
+ char *ptr = (char *) extra + standby_slot_names_config->slot_index_offset;
+ standby_slot_index = (int *) ptr;
+ }
}
/*
@@ -2590,9 +2619,21 @@ StandbySlotsHaveCaughtup(XLogRecPtr wait_for_lsn, int elevel)
XLogRecPtr restart_lsn;
bool invalidated;
bool inactive;
- ReplicationSlot *slot;
+ ReplicationSlot *slot = NULL;
+ int slot_index = standby_slot_index[i];
- slot = SearchNamedReplicationSlot(name, false);
+ /* Get the replication slot using the standby slot index */
+ if (slot_index >= 0)
+ slot = &ReplicationSlotCtl->replication_slots[slot_index];
+
+ /*
+ * If the slot index has not been initialized, or if the slot found by
+ * the index does not match, which means the original slot was dropped,
+ * then we search for the named slot among all replication slots and
+ * update the index if the slot is found.
+ */
+ if (!slot || !slot->in_use || strcmp(name, NameStr(slot->data.name)) != 0)
+ slot = SearchNamedReplicationSlot(name, false);
if (!slot)
{
@@ -2616,6 +2657,8 @@ StandbySlotsHaveCaughtup(XLogRecPtr wait_for_lsn, int elevel)
break;
}
+ standby_slot_index[i] = ReplicationSlotIndex(slot);
+
if (SlotIsLogical(slot))
{
/*
--
2.30.0.windows.2
[application/x-zip-compressed] test_3.zip (139.0K, ../OS0PR01MB571671A91E602BEFD3083EEE942A2@OS0PR01MB5716.jpnprd01.prod.outlook.com/3-test_3.zip)
download
[application/x-zip-compressed] test_2.zip (2.7K, ../OS0PR01MB571671A91E602BEFD3083EEE942A2@OS0PR01MB5716.jpnprd01.prod.outlook.com/4-test_2.zip)
download
[application/x-zip-compressed] test_1.zip (2.9K, ../OS0PR01MB571671A91E602BEFD3083EEE942A2@OS0PR01MB5716.jpnprd01.prod.outlook.com/5-test_1.zip)
download
view thread (3+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: Synchronizing slots from primary to standby
In-Reply-To: <OS0PR01MB571671A91E602BEFD3083EEE942A2@OS0PR01MB5716.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox