Re: Documentation to upgrade logical replication cluster

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(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-09 06:59:51
Message-ID: CAHut+PvokQupz5TQ5x1dXOT++ZpNP-AVX=R76gdtTK4iS2q3Pw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Here are some review comments for patch v7-0001.

======
doc/src/sgml/glossary.sgml

1.
+ <glossentry id="glossary-logical-replication-cluster">
+ <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>
+ </glossentry>

1a.
/instance with/instances with/

~~~

1b.
The description then made me want to look up the glossary definition
of a "publisher instance" and "subscriber instance", but then I was
quite surprised that even "Publisher" and "Subscriber" terms are not
described in the glossary. Should this patch add those, or should we
start another thread for adding them?

======
doc/src/sgml/logical-replication.sgml

2.
+ <para>
+ Migration of logical replication clusters is possible only when all the
+ members of the old logical replication clusters are version 17.0 or later.
+ </para>

Here is where "logical replication clusters" is mentioned. Shouldn't
this first reference be linked to that new to the glossary entry --
e.g. <glossterm linkend="...">logical replication clusters</glossterm>

~~~

3.
+ <para>
+ Following are the prerequisites for <application>pg_upgrade</application>
+ to be able to upgrade the logical slots. If these are not met an error
+ will be reported.
+ </para>

SUGGESTION
The following prerequisites are required for ...

~~~

4.
+ <para>
+ All slots on the old cluster must be usable, i.e., there are no slots
+ whose
+ <link linkend="view-pg-replication-slots">pg_replication_slots</link>.<structfield>conflict_reason</structfield>
+ is not <literal>NULL</literal>.
+ </para>

The double-negative is too tricky "no slots whose ... not NULL", needs
rewording. Maybe it is better to instead use an example as the next
bullet point does.

~~~

5.
+
+ <para>
+ Following are the prerequisites for
<application>pg_upgrade</application> to
+ be able to upgrade the subscriptions. If these are not met an error
+ will be reported.
+ </para>

SUGGESTION
The following prerequisites are required for ...

======
doc/src/sgml/ref/pgupgrade.sgml

6.
+ <note>
+ <para>
+ The steps to upgrade logical replication clusters are not covered here;
+ refer to <xref linkend="logical-replication-upgrade"/> for details.
+ </para>
+ </note>

Maybe here too there should be a link to the glossary term "logical
replication clusters".

======
Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhijie Hou (Fujitsu) 2024-02-09 07:08:01 RE: Synchronizing slots from primary to standby
Previous Message vignesh C 2024-02-09 06:49:28 Re: Race condition in FetchTableStates() breaks synchronization of subscription tables