public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hayato Kuroda (Fujitsu) <[email protected]>
To: 'shveta malik' <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Drouvot, Bertrand <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[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: Ajin Cherian <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Subject: RE: Synchronizing slots from primary to standby
Date: Tue, 19 Dec 2023 13:05:17 +0000
Message-ID: <TY3PR01MB9889213E060DA388FFECAD68F597A@TY3PR01MB9889.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAJpy0uB88wSeBiBb2eGsFusBmLVj7o_bJidViecf35wrktQc=A@mail.gmail.com>
References: <CAJpy0uC+2agRtF3H=n-hW5JkoPfaZkjPXJr==y3_PRE04dQvhw@mail.gmail.com>
<CAA4eK1LxGd8GACR36KdtbVaQeKsV9B5-LuBf2o721x6OZ=_wWg@mail.gmail.com>
<[email protected]>
<CABdArM5zvpFcOF=_0W-E5bjqjrC2MK+EtFN0UfP43zvaFOc8QA@mail.gmail.com>
<CAJpy0uCBHRX-GSKCeVza44kFEC=uTMD_6uzuXXnbUX32Vt-g8Q@mail.gmail.com>
<[email protected]>
<CAJpy0uDiffV66DofhxZr97Sw2U_-ZkdNCjdRt9tKhnf-CxW57Q@mail.gmail.com>
<CAA4eK1+AumKenLjtVW2y4CpxBr_bo_AVZ67RWdDeJFt+Kgrj0A@mail.gmail.com>
<[email protected]>
<CAA4eK1+EeUUT+gnzoHKWm7GqosA2ehT8QyoKtu1jiyGE=wUErw@mail.gmail.com>
<CAJpy0uCAzkua8KAQaLNnYKOJ56x3yJ9kRfDxL8Enp5Li8bzhdQ@mail.gmail.com>
<[email protected]>
<CAJpy0uDU-qaspyU7nNduoqLGjeUT1jVbRyvKMTUvQNp3eY+08Q@mail.gmail.com>
<CAJpy0uBYJh+6vsX2VnecZvc=TUFMY93O9Pob07VzsQQV3cGBWQ@mail.gmail.com>
<CAA4eK1+wN+K3vMOxtE=Lo98aimctiBdozMWaNPG55NUGWW3Gww@mail.gmail.com>
<[email protected]>
<CAJpy0uC-8mrn6jakcFjSVmbJiHZs-Okq8YKxGfrMLPD-2=wOqQ@mail.gmail.com>
<CAA4eK1Ki1O65SyA6ijh-Mq4zpzeh644fCmkrZXMJcQXHNrAw0Q@mail.gmail.com>
<CAJpy0uB3tJCxPCbPHJpf4yeBvnhUsHUEJWtmm8Ha6s6QCBm55w@mail.gmail.com>
<CAJpy0uBgGRJvWYfJD=3XE8OZ0V=X1keOYXxXEkpJqYk00BAzZg@mail.gmail.com>
<CAHut+Psvxs-=j3aCpPVs3e4w78HndCdO-F4bLPzAX70+dgWUuQ@mail.gmail.com>
<CAA4eK1+w9yv+4UZXhiDHZpGDfbeRHYDBu23FwsniS8sYUZeu1w@mail.gmail.com>
<CAJpy0uCv71ynK7zFkKWy3wa7ua+wGw=zoGKy9w=Xq2sq4PqAFQ@mail.gmail.com>
<CAJpy0uD+FcaakEpUp3CVAn+PhmRdkxb8gv4ONZCgPQ7Jm0rBcg@mail.gmail.com>
<CAHut+PsyZQZ1A4XcKw-D=vcTg16pN9Dw0PzE8W_X7Yz_bv00rQ@mail.gmail.com>
<CAA4eK1Kh2cj5vjknAxibpp8Dn+jjVwT+F7oMPT1P861s_ZrDXQ@mail.gmail.com>
<CAJpy0uB88wSeBiBb2eGsFusBmLVj7o_bJidViecf35wrktQc=A@mail.gmail.com>
Dear Shveta,
I resumed to review the patch. I will play more about it, but I can post some
cosmetic comments.
====
walsender.c
01. WalSndWaitForStandbyConfirmation
```
+ sleeptime = WalSndComputeSleeptime(GetCurrentTimestamp());
```
It works well, but I'm not sure whether we should use WalSndComputeSleeptime()
because the function won't be called by walsender.
02.WalSndWaitForStandbyConfirmation
```
+ ConditionVariableTimedSleep(&WalSndCtl->wal_confirm_rcv_cv, sleeptime,
+ WAIT_EVENT_WAL_SENDER_WAIT_FOR_STANDBY_CONFIRMATION)
```
Hmm, is it OK to use the same event as WalSndWaitForWal()? IIUC it should be avoided.
03. WalSndShmemInit()
```
+
+ ConditionVariableInit(&WalSndCtl->wal_confirm_rcv_cv);
```
Unnecessary blank?
~~~~~
050_standby_failover_slots_sync.pl
04. General
My pgperltidy modified your test. Please check.
05.
```
# Create publication on the primary
```
Missing "a" before publication?
06.
```
$subscriber1->init(allows_streaming => 'logical');
...
$subscriber2->init(allows_streaming => 'logical');
```
IIUC, these settings are not needed.
07.
```
my $primary_insert_time = time();
```
The variable is not used.
08.
```
# Stop the standby associated with the specified physical replication slot so
# that the logical replication slot won't receive changes until the standby
# slot's restart_lsn is advanced or the slot is removed from the
# standby_slot_names list
```
Missing comma?
09.
```
$back_q->query_until(qr//,
"SELECT pg_logical_slot_get_changes('test_slot', NULL, NULL);\n");
```
Not sure, should we have to close the back_q connection?
10.
```
# Remove the standby from the standby_slot_names list and reload the
# configuration
$primary->adjust_conf('postgresql.conf', 'standby_slot_names', "''");
$primary->psql('postgres', "SELECT pg_reload_conf()");
```
a.
Missing comma?
b.
I counted and reload function in perl (e.g., `$primary->reload;`) is more often to
be used. Do you have a reason to use pg_reload_conf()?
11.
```
# Now that the standby lsn has advanced, the primary must send the decoded
# changes to the subscription.
$publisher->wait_for_catchup('regress_mysub1');
```
Is the comment correct? I think primary sends data because the GUC is modified.
12.
```
# Put the standby back on the primary_slot_name for the rest of the tests
$primary->adjust_conf('postgresql.conf', 'standby_slot_names', 'sb1_slot');
$primary->restart();
```
Just to confirm - you used restart() here because we must ensure the GUC change is
propagated to all backends, right?
~~~~~
wait_event_names.txt
13.
```
+WAL_SENDER_WAIT_FOR_STANDBY_CONFIRMATION "Waiting for the WAL to be received by physical standby in WAL sender process."
```
But there is a possibility that backend processes may wait with the event, right?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
view thread (108+ 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]
Subject: RE: Synchronizing slots from primary to standby
In-Reply-To: <TY3PR01MB9889213E060DA388FFECAD68F597A@TY3PR01MB9889.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