Re: speed up a logical replica setup

On Thu, 7 Mar 2024 at 18:31, Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>
> Hi,
>
> > Thanks for the feedback. I'm attaching v26 that addresses most of your comments
> > and some issues pointed by Vignesh [1].
>
> I have created a top-up patch v27-0004. It contains additional test
> cases for pg_createsubscriber.
>
> Currently, two testcases (in v27-0004 patch) are failing. These
> failures are related to running pg_createsubscriber on nodes in
> cascade physical replication and are already reported in [1] and [2].
> I think these cases should be fixed. Thoughts?

We can fix these issues, if we are not planning to fix any of them, we
can add documentation for the same.

> The idea of this patch is to keep track of testcases, so that any
> future patch does not break any scenario which has already been worked
> on. These testcases can be used to test in our development process,
> but which test should actually be committed, can be discussed later.
> Thought?

Few comments for v27-0004-Add-additional-testcases.patch:
1) We could use command_fails_like to verify the reason of the error:
+# set max_replication_slots
+$node_p->append_conf('postgresql.conf', 'max_replication_slots = 3');
+$node_p->restart;
+command_fails(
+       [
+               'pg_createsubscriber', '--verbose',
+               '--dry-run', '--pgdata',
+               $node_s->data_dir, '--publisher-server',
+               $node_p->connstr('pg1'), '--socket-directory',
+               $node_s->host, '--subscriber-port',
+               $node_s->port, '--database',
+               'pg1', '--database',
+               'pg2',
+       ],
+       'max_replication_slots are less in number in publisher');

2) Add a comment saying what is being verified
+# set max_replication_slots
+$node_p->append_conf('postgresql.conf', 'max_replication_slots = 3');
+$node_p->restart;
+command_fails(
+       [
+               'pg_createsubscriber', '--verbose',
+               '--dry-run', '--pgdata',
+               $node_s->data_dir, '--publisher-server',
+               $node_p->connstr('pg1'), '--socket-directory',
+               $node_s->host, '--subscriber-port',
+               $node_s->port, '--database',
+               'pg1', '--database',
+               'pg2',
+       ],
+       'max_replication_slots are less in number in publisher');

3) We could rename this file something like
pg_create_subscriber_failure_cases or something better:
 src/bin/pg_basebackup/t/041_tests.pl | 285 +++++++++++++++++++++++++++
 1 file changed, 285 insertions(+)
 create mode 100644 src/bin/pg_basebackup/t/041_tests.pl

diff --git a/src/bin/pg_basebackup/t/041_tests.pl
b/src/bin/pg_basebackup/t/041_tests.pl
new file mode 100644
index 0000000000..2889d60d54
--- /dev/null
+++ b/src/bin/pg_basebackup/t/041_tests.pl
@@ -0,0 +1,285 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group


4) This success case is not required as this would have already been
covered in 040_pg_createsubscriber.pl:
+$node_p->append_conf('postgresql.conf', 'max_replication_slots = 4');
+$node_p->restart;
+command_ok(
+       [
+               'pg_createsubscriber', '--verbose',
+               '--dry-run', '--pgdata',
+               $node_s->data_dir, '--publisher-server',
+               $node_p->connstr('pg1'), '--socket-directory',
+               $node_s->host, '--subscriber-port',
+               $node_s->port, '--database',
+               'pg1', '--database',
+               'pg2',
+       ],
+       'max_replication_slots are accurate on publisher');

5)  We could use command_fails_like to verify the reason of the error:
$node_s->append_conf('postgresql.conf', 'max_replication_slots = 1');
$node_s->restart;
command_fails(
[
'pg_createsubscriber', '--verbose',
'--dry-run', '--pgdata',
$node_s->data_dir, '--publisher-server',
$node_p->connstr('pg1'), '--socket-directory',
$node_s->host, '--subscriber-port',
$node_s->port, '--database',
'pg1', '--database',
'pg2',
],
'max_replication_slots are less in number in subscriber');

6) Add a comment saying what is being verified
$node_s->append_conf('postgresql.conf', 'max_replication_slots = 1');
$node_s->restart;
command_fails(
[
'pg_createsubscriber', '--verbose',
'--dry-run', '--pgdata',
$node_s->data_dir, '--publisher-server',
$node_p->connstr('pg1'), '--socket-directory',
$node_s->host, '--subscriber-port',
$node_s->port, '--database',
'pg1', '--database',
'pg2',
],
'max_replication_slots are less in number in subscriber');

7) This success case is not required as this would have already been
covered in 040_pg_createsubscriber.pl:
$node_s->append_conf('postgresql.conf', 'max_replication_slots = 2');
$node_s->restart;
command_ok(
[
'pg_createsubscriber', '--verbose',
'--dry-run', '--pgdata',
$node_s->data_dir, '--publisher-server',
$node_p->connstr('pg1'), '--socket-directory',
$node_s->host, '--subscriber-port',
$node_s->port, '--database',
'pg1', '--database',
'pg2',
],
'max_replication_slots are less in number in subscriber');

8) We could use command_fails_like to verify the reason of the error:
# set wal_level on publisher
$node_p->append_conf('postgresql.conf', 'wal_level = \'replica\'');
$node_p->restart;
command_fails(
[
'pg_createsubscriber', '--verbose',
'--dry-run', '--pgdata',
$node_s->data_dir, '--publisher-server',
$node_p->connstr('pg1'), '--socket-directory',
$node_s->host, '--subscriber-port',
$node_s->port, '--database',
'pg1', '--database',
'pg2',
],
'wal_level must be logical');

9) Add a comment saying what is being verified
# set wal_level on publisher
$node_p->append_conf('postgresql.conf', 'wal_level = \'replica\'');
$node_p->restart;
command_fails(
[
'pg_createsubscriber', '--verbose',
'--dry-run', '--pgdata',
$node_s->data_dir, '--publisher-server',
$node_p->connstr('pg1'), '--socket-directory',
$node_s->host, '--subscriber-port',
$node_s->port, '--database',
'pg1', '--database',
'pg2',
],
'wal_level must be logical');

10) This success case is not required as this would have already been
covered in 040_pg_createsubscriber.pl:
$node_p->append_conf('postgresql.conf', 'wal_level = \'logical\'');
$node_p->restart;
command_ok(
[
'pg_createsubscriber', '--verbose',
'--dry-run', '--pgdata',
$node_s->data_dir, '--publisher-server',
$node_p->connstr('pg1'), '--socket-directory',
$node_s->host, '--subscriber-port',
$node_s->port, '--database',
'pg1', '--database',
'pg2',
],
'wal_level is logical');

11) We could use command_fails_like to verify the reason of the error:
# set max_wal_senders on publisher
$node_p->append_conf('postgresql.conf', 'max_wal_senders = 2');
$node_p->restart;
command_fails(
[
'pg_createsubscriber', '--verbose',
'--dry-run', '--pgdata',
$node_s->data_dir, '--publisher-server',
$node_p->connstr('pg1'), '--socket-directory',
$node_s->host, '--subscriber-port',
$node_s->port, '--database',
'pg1', '--database',
'pg2',
],
'max_wal_senders is not sufficient');

12) Add a comment saying what is being verified:
# set max_wal_senders on publisher
$node_p->append_conf('postgresql.conf', 'max_wal_senders = 2');
$node_p->restart;
command_fails(
[
'pg_createsubscriber', '--verbose',
'--dry-run', '--pgdata',
$node_s->data_dir, '--publisher-server',
$node_p->connstr('pg1'), '--socket-directory',
$node_s->host, '--subscriber-port',
$node_s->port, '--database',
'pg1', '--database',
'pg2',
],
'max_wal_senders is not sufficient');

13) This success case is not required as this would have already been
covered in 040_pg_createsubscriber.pl:
$node_p->append_conf('postgresql.conf', 'max_wal_senders = 3');
$node_p->restart;
command_ok(
[
'pg_createsubscriber', '--verbose',
'--dry-run', '--pgdata',
$node_s->data_dir, '--publisher-server',
$node_p->connstr('pg1'), '--socket-directory',
$node_s->host, '--subscriber-port',
$node_s->port, '--database',
'pg1', '--database',
'pg2',
],
'max_wal_senders is sufficient');

14) This sleep is not required
# max_worker_processes on subscriber
$node_p->append_conf('postgresql.conf', 'max_worker_processes = 2');
$node_p->restart;
sleep 1;
$node_s->append_conf('postgresql.conf', 'max_worker_processes = 2');
$node_s->restart;

15) The similar comments exist in other places also, I'm not repeating them.

Regards,
Vignesh