Hi, On Mon, Jul 10, 2023 at 02:37:24PM -0700, Nikolay Samokhvalov wrote: > On Mon, Jul 10, 2023 at 2:02 PM Michael Banck <mba...@gmx.net> wrote: > > > +++ b/doc/src/sgml/ref/pgupgrade.sgml > > > @@ -380,22 +380,28 @@ NET STOP postgresql-&majorversion; > > > </para> > > > > > > <para> > > > - Streaming replication and log-shipping standby servers can > > > + Streaming replication and log-shipping standby servers must > > > remain running until a later step. > > > </para> > > > </step> > > > > > > - <step> > > > + <step id="pgupgrade-step-prepare-standbys"> > > > > > > <para> > > > - If you are upgrading standby servers using methods outlined in > > section <xref > > > - linkend="pgupgrade-step-replicas"/>, verify that the old standby > > > - servers are caught up by running > > <application>pg_controldata</application> > > > - against the old primary and standby clusters. Verify that the > > > - <quote>Latest checkpoint location</quote> values match in all > > clusters. > > > - (There will be a mismatch if old standby servers were shut down > > > - before the old primary or if the old standby servers are still > > running.) > > > + If you are upgrading standby servers using methods outlined in > > > + <xref linkend="pgupgrade-step-replicas"/>, > > > > You dropped the "section" before the xref, I think that should be kept > > around. > > Seems to be a problem in discussing source code that looks quite different > than the final result. > > In the result – the docs – we currently have "section Step 9", looking > weird. I still think it's good to remove it. We also have "in Step 17 > below" (without the extra word "section") in a different place on the same > page.
Ok. > > > + ensure that they were > > running when > > > + you shut down the primaries in the previous step, so all the > > latest changes > > > > You talk of primaries in plural here, that is a bit weird for PostgreSQL > > documentation. > > The same docs already discuss two primaries ("8. Stop both primaries"), but > I agree it might look confusing if you read only a part of the doc, jumping > into middle of it, like I do all the time when using the docs in "check the > reference" style. [...] > > I think this should be something like "ensure both that the primary is > > shut down and that the standbys have received all the changes". > > Well, we have two primary servers – old and new. I tried to clarify it in > the new version. Yeah sorry about that, I think I should have first have coffee and/or slept over this review before sending it. Maybe one reason why I was confused is beause I consider a "primary" more like a full server/VM, not necessarily a database instance (though one could of course have a primary/seconday pair on the same host). Possibly "primary instances" or something might be clearer, but I think I should re-read the whole section first before commenting further. Michael