From: Maxim Michkov <m.michkov@arenadata.io>
To: Heikki Linnakangas <hlinnaka@iki.fi>
To: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #18575: Sometimes pg_rewind mistakenly assumes that nothing needs to be done.
Date: Tue, 02 Sep 2025 17:12:14 +0300
Message-ID: <a4896a62ceab7167824d48b8f0afe01aef6140aa.camel@arenadata.io> (raw)
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>
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
> unmodified, in a state such that when you restart it, it will replay
> all
> the WAL it has locally in pg_wal first, before connecting to the
> primary. Even though the target is a direct ancestor of the source
> and
> hence it *can* follow the WAL to the source's position without
> rewinding, it doesn't mean that it *will* actually do so.
>
> The attached changes it so that it updates the control file in that
> case, setting minRecoveryPoint and minRecoveryPointTLI to point to
> the
> source's current WAL position. That way, when you start it up, it
> will
> follow the timeline history to reach that point. (This requires
> fixing
> issue 2, because otherwise it still won't follow the history
> correctly
> 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) !=
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.
Attachments:
[text/x-patch] 0001-Tap-test-for-minRecoveryPoint-matching-switchpoint.patch (2.5K, ../a4896a62ceab7167824d48b8f0afe01aef6140aa.camel@arenadata.io/2-0001-Tap-test-for-minRecoveryPoint-matching-switchpoint.patch)
download | inline diff:
From 54976b5f5f838ccaaea5835afe78987267685e64 Mon Sep 17 00:00:00 2001
From: Maksim Michkov <m.michkov@arenadata.io>
Date: Tue, 2 Sep 2025 14:08:14 +0000
Subject: [PATCH] Tap test for minRecoveryPoint matching switchpoint
---
src/bin/pg_rewind/t/010_target_is_ancestor.pl | 22 +++++--------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/src/bin/pg_rewind/t/010_target_is_ancestor.pl b/src/bin/pg_rewind/t/010_target_is_ancestor.plindex 5f3940cb027..4bc1510305b 100644--- a/src/bin/pg_rewind/t/010_target_is_ancestor.pl+++ b/src/bin/pg_rewind/t/010_target_is_ancestor.pl@@ -52,17 +52,8 @@ $orig_node->backup_fs_cold('backup_B');
# Restore backup B to create Timeline 2
my $node_2 = PostgreSQL::Test::Cluster->new('node_2');
-$node_2->init_from_backup($orig_node, 'backup_B', has_streaming => 1);+$node_2->init_from_backup($orig_node, 'backup_B', has_restoring => 1, standby => 0);
$node_2->start;
-$node_2->promote;-$node_2->safe_psql('postgres', "INSERT INTO test_tab VALUES ('in node_2')");-$node_2->stop;--$orig_node->start;-$orig_node->safe_psql('postgres',- "INSERT INTO test_tab VALUES ('this will be lost on rewind')");-$orig_node->stop;-$orig_node->backup_fs_cold('backup_C');
my $source_node = $node_2;
my $source_pgdata = $source_node->data_dir;
@@ -80,12 +71,14 @@ sub rewind_from_backup
# Keep a temporary postgresql.conf or it would be overwritten during the rewind.
copy("$target_pgdata/postgresql.conf", "$tmp_folder/postgresql.conf.tmp");
+ my $source_connstr = $source_node->connstr();+
command_checks_all(
[
'pg_rewind',
"--debug",
"--no-sync",
- "--source-pgdata=$source_pgdata",+ "--source-server=$source_connstr",
"--target-pgdata=$target_pgdata"
],
0,
@@ -109,7 +102,6 @@ sub rewind_from_backup
'postgresql.conf', qq(
primary_conninfo = '$node_2_connstr'
));
- $node_2->start;
$target_node->set_standby_mode();
$target_node->start;
@@ -117,17 +109,13 @@ primary_conninfo = '$node_2_connstr'
my $result =
$target_node->safe_psql('postgres', "SELECT * FROM test_tab");
is( $result, qq(at the beginning
-after backup A-in node_2),+after backup A),
"check query after rewind from $backup");
$target_node->stop;
$node_2->stop;
}
-rewind_from_backup('backup_C',- [qr/pg_rewind: rewinding from last common checkpoint at .* on timeline 1/]);-rewind_from_backup('backup_B', [qr/pg_rewind: no rewind required/]);
rewind_from_backup('backup_A', [qr/pg_rewind: no rewind required/]);
done_testing();
--
2.34.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: m.michkov@arenadata.io, hlinnaka@iki.fi, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18575: Sometimes pg_rewind mistakenly assumes that nothing needs to be done.
In-Reply-To: <a4896a62ceab7167824d48b8f0afe01aef6140aa.camel@arenadata.io>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox