MIME-Version: 1.0
Disposition-Notification-To: nik@postgres.ai
From: Nikolay Samokhvalov <nik@postgres.ai>
Date: Thu, 29 Jun 2023 10:50:12 -0700
Message-ID: <CAM527d8heqkjG5VrvjU3Xjsqxg41ufUyabD9QZccdAxnpbRH-Q@mail.gmail.com>
Subject: pg_upgrade instructions involving "rsync --size-only" might lead to
 standby corruption?
To: pgsql-hackers@postgresql.org
Content-Type: multipart/alternative; boundary="000000000000a9898e05ff4855dc"
Archived-At: <https://www.postgresql.org/message-id/CAM527d8heqkjG5VrvjU3Xjsqxg41ufUyabD9QZccdAxnpbRH-Q%40mail.gmail.com>
Precedence: bulk

--000000000000a9898e05ff4855dc
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi!

(posting this to -hackers rather than to -docs since it seems a deeper
problem than just adjusting the docs)

I recently observed a case with standby corruption after upgrading pg12 to
pg14, which was presented in the form of XX001 errors on the new cluster's
standby nodes. e.g.:
  ERROR: missing chunk number 0 for toast value 3228893903 in
pg_toast_79504413

Comparing the content of the data directory and checking files with md5sum,
I noticed that some files for some TOAST index have different content on
new standby nodes compared to the new primary =E2=80=93 and interesting was=
 the
fact all standbys had the same content. Just different compared to the
primary.

We used the "rsync --size-only" snippet from the docs
https://www.postgresql.org/docs/current/pgupgrade.html to upgrade standbys.

With "--size-only", 1 GiB files for tables and indexes obviously cannot be
reliably synchronized. In our case, we perform additional steps involving
logical replication, advancing primary to certain LSN position -- and
during that, we keep standbys down. This explains the increased corruption
risks. But I think these risks are present for those who just follow the
steps in the docs as is, and probably some fixes or improvements are needed
here.

The main question: why do we consider "rsync --size-only" as reliable in
the general case? May standby corruption happen if we use follow steps from
https://www.postgresql.org/docs/current/pgupgrade.html?

Considering several general situations:
1. For streaming replication:
    a. if we shut down the primary first, based on the code in walsender.c
defining how shutdown even is handled, replicas should receive all the
changes?
    b. if shut down standbys first (might be preferred if we run cluster
under Patroni control, to avoid unnecessary failover), then some changes
from the primary won't be received by standbys =E2=80=93 and we do have sta=
ndby
corruption risks
2. For replication based on WAL shipping, I don't think we can guarantee
that all changes are propagated to standbys.

The docs also have this:

> 9. Prepare for standby server upgrades
> If you are upgrading standby servers using methods outlined in section
Step 11, verify that the old standby servers are caught up by running
pg_controldata against the old primary and standby clusters. Verify that
the =E2=80=9CLatest checkpoint location=E2=80=9D values match in all cluste=
rs. (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.) Also, make sure wal_level
is not set to minimal in the postgresql.conf file on the new primary
cluster.

=E2=80=93 admitting that there might be mismatch. But if there is mismatch,=
 rsync
--size-only is not going to help synchronize properly, right?

I was thinking about how to improve here, some ideas:
- "rsync --checksum" doesn't seem to be a good idea, it's, unfortunately,
very, very slow, though it would be the most reliable approach (but since
it's slow, I guess it's not worth even mentioning, crossing this out)
- we could remove "--size-only" and rely on default rsync behavior =E2=80=
=93
checking size and modification time; but how reliable would it be in
general case?
- make the step verifying =E2=80=9CLatest checkpoint location=E2=80=9D *aft=
er* shutting
down all nodes as mandatory, with instructions on how to avoid mismatch:
e.g., shut down primary first, disabling automated failover software, if
any, then run pg_controldata on standbys while they are running, and on
primary while it's already shut down (probably, different instructions are
needed for WAL shipping and streaming cases)
- probably, we should always run "rsync --checksum" for pg_wal
- I think, it's time to provide a snippet to run "rsync" in multiple
threads. A lot of installations today have many vCPUs and fast SSDs, and
running single-threaded rsync seems to be very slow (especially if we do
need to move away from "--size-only"). If it makes sense, I could come up
with some patch proposal for the docs
-  it's probably time to implement support for standby upgrade in
pg_upgrade itself, finding some way to take care of standbys and moving
away from the need to run rsync or to rebuild standby nodes? Although, this
is just a raw idea without a proper proposal yet.

Does this make sense or I'm missing something and the current docs describe
a reliable process? (As I said, we have deviated from the process, to
involve logical replication, so I'm not 100% sure I'm right suspecting the
original procedure in having standby corruption risks.)

Thanks,
Nikolay Samokhvalov
Founder, Postgres.ai

--000000000000a9898e05ff4855dc
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Hi!</div><div><br></div><div>(posting this to -hacker=
s rather than to -docs since it seems a deeper problem than just adjusting =
the docs)</div><div><br></div>I recently observed a case with standby corru=
ption after upgrading pg12 to pg14, which was presented in the form of XX00=
1 errors on the new cluster&#39;s standby nodes. e.g.:<div>=C2=A0 ERROR: mi=
ssing chunk number 0 for toast value 3228893903 in pg_toast_79504413<div><b=
r></div><div>Comparing the content of the data directory and checking files=
 with md5sum, I noticed that some files for some TOAST index have different=
 content on new standby nodes compared to the new primary =E2=80=93 and int=
eresting was the fact all standbys had the same content. Just different com=
pared to the primary.</div><div><br></div><div>We used the &quot;rsync --si=
ze-only&quot; snippet from the docs <a href=3D"https://www.postgresql.org/d=
ocs/current/pgupgrade.html">https://www.postgresql.org/docs/current/pgupgra=
de.html</a> to upgrade standbys.</div><div><br></div><div>With &quot;--size=
-only&quot;, 1 GiB files for tables and indexes obviously cannot be reliabl=
y synchronized. In our case, we perform additional steps involving logical =
replication, advancing primary to certain LSN position -- and during that, =
we keep standbys down. This explains the increased corruption risks. But I =
think these risks are present for those who just follow the steps in the do=
cs as is, and probably some fixes or improvements are needed here.</div><di=
v><br></div><div>The main question: why do we consider &quot;rsync --size-o=
nly&quot; as reliable in the general case? May standby corruption happen if=
 we use follow steps from <a href=3D"https://www.postgresql.org/docs/curren=
t/pgupgrade.html">https://www.postgresql.org/docs/current/pgupgrade.html</a=
>?</div><div><br></div><div>Considering several general situations:</div><d=
iv>1. For streaming replication:</div><div>=C2=A0 =C2=A0 a. if we shut down=
 the primary first, based on the code in walsender.c defining how shutdown =
even is handled, replicas should receive all the changes?</div><div>=C2=A0 =
=C2=A0 b. if shut down standbys first (might be preferred if we run cluster=
 under Patroni control, to avoid unnecessary failover), then some changes f=
rom the primary won&#39;t be received by standbys =E2=80=93 and we do have =
standby corruption risks=C2=A0</div><div>2. For replication based on WAL sh=
ipping, I don&#39;t think we can guarantee that all changes are propagated =
to standbys.</div><div><br></div><div>The docs also have this:</div><div><b=
r></div><div>&gt; 9. Prepare for standby server upgrades=C2=A0</div><div>&g=
t; If you are upgrading standby servers using methods outlined in section S=
tep 11, verify that the old standby servers are caught up by running pg_con=
troldata against the old primary and standby clusters. Verify that the =E2=
=80=9CLatest checkpoint location=E2=80=9D values match in all clusters. (Th=
ere will be a mismatch if old standby servers were shut down before the old=
 primary or if the old standby servers are still running.) Also, make sure =
wal_level is not set to minimal in the postgresql.conf file on the new prim=
ary cluster.<br></div><div><br></div><div>=E2=80=93 admitting that there mi=
ght be mismatch. But if there is mismatch, rsync --size-only is not going t=
o help synchronize properly, right?</div><div><br></div><div>I was thinking=
 about how to improve here, some ideas:</div><div>- &quot;rsync --checksum&=
quot; doesn&#39;t seem to be a good idea, it&#39;s, unfortunately, very, ve=
ry slow, though it would be the most reliable approach (but since it&#39;s =
slow, I guess it&#39;s not worth even mentioning, crossing this out)</div><=
div>- we could remove &quot;--size-only&quot; and rely on default rsync beh=
avior =E2=80=93 checking size and modification time; but how reliable would=
 it be in general case?</div><div>- make the step verifying =E2=80=9CLatest=
 checkpoint location=E2=80=9D *after* shutting down all nodes as mandatory,=
 with instructions on how to avoid mismatch: e.g., shut down primary first,=
 disabling automated failover software, if any, then run pg_controldata on =
standbys while they are running, and on primary while it&#39;s already shut=
 down (probably, different instructions are needed for WAL shipping and str=
eaming cases)=C2=A0</div><div>- probably, we should always run &quot;rsync =
--checksum&quot; for pg_wal=C2=A0</div><div>- I think, it&#39;s time to pro=
vide a snippet to run &quot;rsync&quot; in multiple threads. A lot of insta=
llations today have many vCPUs and fast SSDs, and running single-threaded r=
sync seems to be very slow (especially if we do need to move away from &quo=
t;--size-only&quot;). If it makes sense, I could come up with some patch pr=
oposal for the docs</div><div>-=C2=A0 it&#39;s probably time to implement s=
upport for standby upgrade in pg_upgrade itself, finding some way to take c=
are of standbys and moving away from the need to run rsync or to rebuild st=
andby nodes? Although, this is just a raw idea without a proper proposal ye=
t.</div><div><br></div><div>Does this make sense or I&#39;m missing somethi=
ng and the current docs describe a reliable process? (As I said, we have de=
viated from the process, to involve logical replication, so I&#39;m not 100=
% sure I&#39;m right suspecting the original procedure in having standby co=
rruption risks.)</div><div><br></div><div>Thanks,<br>Nikolay Samokhvalov<br=
>Founder, Postgres.ai<br></div></div></div>

--000000000000a9898e05ff4855dc--