Re: Documentation to upgrade logical replication cluster

From: vignesh C <vignesh21(at)gmail(dot)com>
To: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: Peter Smith <smithpb2250(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Subject: Re: Documentation to upgrade logical replication cluster
Date: 2024-02-01 09:20:10
Message-ID: CALDaNm2JDD_qSfgnXAMP1N7+3f9fz1xx-siNcBSYfd+Ep4zKyg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 31 Jan 2024 at 11:42, Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear Vignesh,
>
> Thanks for updating the patch! Here are my comments for v6.
>
> 01.
> ```
> + <glossterm>Logical replication cluster</glossterm>
> + <glossdef>
> + <para>
> + A set of publisher and subscriber instance with publisher instance
> + replicating changes to the subscriber instance.
> + </para>
> + </glossdef>
> ```
>
> Should we say 1:N relationship is allowed?

I felt this need not be mentioned here, just wanted to give an
indication wherever this terminology is used, it means a set of
publisher and subscriber instances. Detail information should be added
in the logical replication related pages

> 02.
> ```
> @@ -70,6 +70,7 @@ PostgreSQL documentation
> pg_upgrade supports upgrades from 9.2.X and later to the current
> major release of <productname>PostgreSQL</productname>, including snapshot and beta releases.
> </para>
> +
> </refsect1>
> ```
>
> Unnecessary blank.

Removed it.

> 03.
> ```
> <para>
> - These are the steps to perform an upgrade
> - with <application>pg_upgrade</application>:
> + Below are the steps to perform an upgrade
> + with <application>pg_upgrade</application>.
> </para>
> ```
>
> I'm not sure it should be included in this patch.

This is not required in this patch, removed it.

> 04.
> ```
> + If the old primary is prior to version 17.0, then no slots on the primary
> + are copied to the new standby, so all the slots on the old standby must
> + be recreated manually.
> ```
>
> I think that "all the slots on the old standby" must be created manually in any
> cases. Therefore, the preposition ", so" seems not correct.

I felt that this change is not related to this patch. I'm removing
these changes from the patch. Let's handle rephrasing of the base code
change in a separate thread.

> 05.
> ```
> If the old primary is version 17.0 or later, then
> + only logical slots on the primary are copied to the new standby, but
> + other slots on the old standby are not copied, so must be recreated
> + manually.
> ```
>
> How about replacing this paragraph to below?
>
> ```
> All the slots on the old standby must be recreated manually. If the old primary
> is version 17.0 or later, then only logical slots on the primary are copied to the
> new standby.
> ```

I felt that this change is not related to this patch. I'm removing
these changes from the patch. Let's handle rephrasing of the base code
change in a separate thread.

Thanks for the comments, the attached v7 version patch has the changes
for the same.

Regards,
Vignesh

Attachment Content-Type Size
v7-0001-Documentation-for-upgrading-logical-replication-c.patch text/x-patch 33.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2024-02-01 09:34:19 Re: Why is src/test/modules/committs/t/002_standby.pl flaky?
Previous Message Amit Kapila 2024-02-01 09:05:15 Re: Synchronizing slots from primary to standby