Message-ID: <a4896a62ceab7167824d48b8f0afe01aef6140aa.camel@arenadata.io>
Subject: Re: BUG #18575: Sometimes pg_rewind mistakenly assumes that nothing
 needs to be done.
From: Maxim Michkov <m.michkov@arenadata.io>
To: Heikki Linnakangas <hlinnaka@iki.fi>, "pgsql-bugs@lists.postgresql.org"
	 <pgsql-bugs@lists.postgresql.org>
Date: Tue, 02 Sep 2025 17:12:14 +0300
In-Reply-To: <fe7af87d-b9bf-481a-902c-089c49aa911d@iki.fi>
References: <18575-1b9b3b60e2a480de@postgresql.org>
	 <092dd515-b7b4-4fd0-8407-ceca2f02f6ec@iki.fi>
	 <3382141723103572@mail.yandex.ru>
	 <e08235e3-b216-4bf3-8a9a-8ea819ae105e@iki.fi>
	 <188271723109777@mail.yandex.com>
	 <3a626a21-319b-44e9-a2b5-94b8a93f9cd8@iki.fi>
	 <31111723111906@mail.yandex.com> <1760421723118253@mail.yandex.com>
	 <2171536a-8227-4e53-ac47-33b69093b61d@iki.fi>
	 <fe7af87d-b9bf-481a-902c-089c49aa911d@iki.fi>
Content-Type: multipart/mixed; boundary="=-CONP9yvcx3c/SoZ0MWte"
User-Agent: Evolution 3.56.2 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/a4896a62ceab7167824d48b8f0afe01aef6140aa.camel%40arenadata.io>
Precedence: bulk

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

On Fri, 2024-08-09 at 18:26 +0300, Heikki Linnakangas wrote:
> 3. When pg_rewind has nothing to do, the target server is left=20
> unmodified, in a state such that when you restart it, it will replay
> all=20
> the WAL it has locally in pg_wal first, before connecting to the=20
> primary. Even though the target is a direct ancestor of the source
> and=20
> hence it *can* follow the WAL to the source's position without=20
> rewinding, it doesn't mean that it *will* actually do so.
>=20
> The attached=C2=A0 changes it so that it updates the control file in that=
=20
> case, setting minRecoveryPoint and minRecoveryPointTLI to point to
> the=20
> source's current WAL position. That way, when you start it up, it
> will=20
> follow the timeline history to reach that point. (This requires
> fixing=20
> issue 2, because otherwise it still won't follow the history
> correctly=20
> to reach the minRecoveryPointTLI)

Hello, I am currently researching a very similar issue. It seems like
your patches work correctly (thank you for those), but I've found a
weird edge case for patch 0004.

Basically, if you don't do anything to the primary after promotion (so
that its last checkpoint is *exactly* at the switchpoint), pg_rewind
with patch 0004 sets minRecoveryPoint to the exact LSN of the
switchpoint. After that the standby won't start, producing errors like
`requested timeline 2 does not contain minimum recovery point 0/4711F38
on timeline 2`.

This happens because of this check in xlogrecovery.c:
```
if (!XLogRecPtrIsInvalid(ControlFile->minRecoveryPoint) &&
	tliOfPointInHistory(ControlFile->minRecoveryPoint - 1,
expectedTLEs) !=3D
	ControlFile->minRecoveryPointTLI)
	ereport(FATAL, /* ... */);
```
Because it checks TLI of minRecoveryPoint - 1, it expects to see the
timeline before the switch (timeline 1), but we actually want it to go
to timeline 2.
It seems like minRecoveryPoint is supposed to indicate minimum allowed
end of WAL, so when setting it to ensure some WAL is processed (like
the checkpoint WAL on another timeline) we have to set it to LSN+1
instead. Do you think this is the correct fix, or if instead -1 in
xlogrecovery.c should be removed?

Attached is a patch to 010 TAP test that reproduces this behavior.

--=-CONP9yvcx3c/SoZ0MWte
Content-Disposition: attachment;
	filename="0001-Tap-test-for-minRecoveryPoint-matching-switchpoint.patch"
Content-Type: text/x-patch;
	name="0001-Tap-test-for-minRecoveryPoint-matching-switchpoint.patch";
	charset="UTF-8"
Content-Transfer-Encoding: base64

RnJvbSA1NDk3NmI1ZjVmODM4Y2NhYWVhNTgzNWFmZTc4OTg3MjY3Njg1ZTY0IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBNYWtzaW0gTWljaGtvdiA8bS5taWNoa292QGFyZW5hZGF0YS5p
bz4KRGF0ZTogVHVlLCAyIFNlcCAyMDI1IDE0OjA4OjE0ICswMDAwClN1YmplY3Q6IFtQQVRDSF0g
VGFwIHRlc3QgZm9yIG1pblJlY292ZXJ5UG9pbnQgbWF0Y2hpbmcgc3dpdGNocG9pbnQKCi0tLQog
c3JjL2Jpbi9wZ19yZXdpbmQvdC8wMTBfdGFyZ2V0X2lzX2FuY2VzdG9yLnBsIHwgMjIgKysrKyst
LS0tLS0tLS0tLS0tLQogMSBmaWxlIGNoYW5nZWQsIDUgaW5zZXJ0aW9ucygrKSwgMTcgZGVsZXRp
b25zKC0pCgpkaWZmIC0tZ2l0IGEvc3JjL2Jpbi9wZ19yZXdpbmQvdC8wMTBfdGFyZ2V0X2lzX2Fu
Y2VzdG9yLnBsIGIvc3JjL2Jpbi9wZ19yZXdpbmQvdC8wMTBfdGFyZ2V0X2lzX2FuY2VzdG9yLnBs
CmluZGV4IDVmMzk0MGNiMDI3Li40YmMxNTEwMzA1YiAxMDA2NDQKLS0tIGEvc3JjL2Jpbi9wZ19y
ZXdpbmQvdC8wMTBfdGFyZ2V0X2lzX2FuY2VzdG9yLnBsCisrKyBiL3NyYy9iaW4vcGdfcmV3aW5k
L3QvMDEwX3RhcmdldF9pc19hbmNlc3Rvci5wbApAQCAtNTIsMTcgKzUyLDggQEAgJG9yaWdfbm9k
ZS0+YmFja3VwX2ZzX2NvbGQoJ2JhY2t1cF9CJyk7CiAKICMgUmVzdG9yZSBiYWNrdXAgQiB0byBj
cmVhdGUgVGltZWxpbmUgMgogbXkgJG5vZGVfMiA9IFBvc3RncmVTUUw6OlRlc3Q6OkNsdXN0ZXIt
Pm5ldygnbm9kZV8yJyk7Ci0kbm9kZV8yLT5pbml0X2Zyb21fYmFja3VwKCRvcmlnX25vZGUsICdi
YWNrdXBfQicsIGhhc19zdHJlYW1pbmcgPT4gMSk7Ciskbm9kZV8yLT5pbml0X2Zyb21fYmFja3Vw
KCRvcmlnX25vZGUsICdiYWNrdXBfQicsIGhhc19yZXN0b3JpbmcgPT4gMSwgc3RhbmRieSA9PiAw
KTsKICRub2RlXzItPnN0YXJ0OwotJG5vZGVfMi0+cHJvbW90ZTsKLSRub2RlXzItPnNhZmVfcHNx
bCgncG9zdGdyZXMnLCAiSU5TRVJUIElOVE8gdGVzdF90YWIgVkFMVUVTICgnaW4gbm9kZV8yJyki
KTsKLSRub2RlXzItPnN0b3A7Ci0KLSRvcmlnX25vZGUtPnN0YXJ0OwotJG9yaWdfbm9kZS0+c2Fm
ZV9wc3FsKCdwb3N0Z3JlcycsCi0JIklOU0VSVCBJTlRPIHRlc3RfdGFiIFZBTFVFUyAoJ3RoaXMg
d2lsbCBiZSBsb3N0IG9uIHJld2luZCcpIik7Ci0kb3JpZ19ub2RlLT5zdG9wOwotJG9yaWdfbm9k
ZS0+YmFja3VwX2ZzX2NvbGQoJ2JhY2t1cF9DJyk7CiAKIG15ICRzb3VyY2Vfbm9kZSA9ICRub2Rl
XzI7CiBteSAkc291cmNlX3BnZGF0YSA9ICRzb3VyY2Vfbm9kZS0+ZGF0YV9kaXI7CkBAIC04MCwx
MiArNzEsMTQgQEAgc3ViIHJld2luZF9mcm9tX2JhY2t1cAogCSMgS2VlcCBhIHRlbXBvcmFyeSBw
b3N0Z3Jlc3FsLmNvbmYgb3IgaXQgd291bGQgYmUgb3ZlcndyaXR0ZW4gZHVyaW5nIHRoZSByZXdp
bmQuCiAJY29weSgiJHRhcmdldF9wZ2RhdGEvcG9zdGdyZXNxbC5jb25mIiwgIiR0bXBfZm9sZGVy
L3Bvc3RncmVzcWwuY29uZi50bXAiKTsKIAorCW15ICRzb3VyY2VfY29ubnN0ciA9ICRzb3VyY2Vf
bm9kZS0+Y29ubnN0cigpOworCiAJY29tbWFuZF9jaGVja3NfYWxsKAogCQlbCiAJCQkncGdfcmV3
aW5kJywKIAkJCSItLWRlYnVnIiwKIAkJCSItLW5vLXN5bmMiLAotCQkJIi0tc291cmNlLXBnZGF0
YT0kc291cmNlX3BnZGF0YSIsCisJCQkiLS1zb3VyY2Utc2VydmVyPSRzb3VyY2VfY29ubnN0ciIs
CiAJCQkiLS10YXJnZXQtcGdkYXRhPSR0YXJnZXRfcGdkYXRhIgogCQldLAogCQkwLApAQCAtMTA5
LDcgKzEwMiw2IEBAIHN1YiByZXdpbmRfZnJvbV9iYWNrdXAKIAkJJ3Bvc3RncmVzcWwuY29uZics
IHFxKAogcHJpbWFyeV9jb25uaW5mbyA9ICckbm9kZV8yX2Nvbm5zdHInCiApKTsKLQkkbm9kZV8y
LT5zdGFydDsKIAkkdGFyZ2V0X25vZGUtPnNldF9zdGFuZGJ5X21vZGUoKTsKIAkkdGFyZ2V0X25v
ZGUtPnN0YXJ0OwogCkBAIC0xMTcsMTcgKzEwOSwxMyBAQCBwcmltYXJ5X2Nvbm5pbmZvID0gJyRu
b2RlXzJfY29ubnN0cicKIAlteSAkcmVzdWx0ID0KIAkgICR0YXJnZXRfbm9kZS0+c2FmZV9wc3Fs
KCdwb3N0Z3JlcycsICJTRUxFQ1QgKiBGUk9NIHRlc3RfdGFiIik7CiAJaXMoICRyZXN1bHQsIHFx
KGF0IHRoZSBiZWdpbm5pbmcKLWFmdGVyIGJhY2t1cCBBCi1pbiBub2RlXzIpLAorYWZ0ZXIgYmFj
a3VwIEEpLAogCQkiY2hlY2sgcXVlcnkgYWZ0ZXIgcmV3aW5kIGZyb20gJGJhY2t1cCIpOwogCSR0
YXJnZXRfbm9kZS0+c3RvcDsKIAogCSRub2RlXzItPnN0b3A7CiB9CiAKLXJld2luZF9mcm9tX2Jh
Y2t1cCgnYmFja3VwX0MnLAotCVtxci9wZ19yZXdpbmQ6IHJld2luZGluZyBmcm9tIGxhc3QgY29t
bW9uIGNoZWNrcG9pbnQgYXQgLiogb24gdGltZWxpbmUgMS9dKTsKLXJld2luZF9mcm9tX2JhY2t1
cCgnYmFja3VwX0InLCBbcXIvcGdfcmV3aW5kOiBubyByZXdpbmQgcmVxdWlyZWQvXSk7CiByZXdp
bmRfZnJvbV9iYWNrdXAoJ2JhY2t1cF9BJywgW3FyL3BnX3Jld2luZDogbm8gcmV3aW5kIHJlcXVp
cmVkL10pOwogCiBkb25lX3Rlc3RpbmcoKTsKLS0gCjIuMzQuMQoK


--=-CONP9yvcx3c/SoZ0MWte--