From bddbb33dfe6a57373bd76f06d928530603eecd64 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 9 Aug 2024 13:30:55 +0300 Subject: [PATCH 2/4] Remove bogus assertion in pg_rewind It's entirely possible that the calculated "point of divergence" is before the end of WAL on the target, because when we calculate the point of divergence, we assume that the target's WAL extends to infinity on its current timeline. It might be more clear to calculate the end of WAL first, and if the target is a direct ancestor of the source, consider the end of target WAL as the point of divergence. However, it doesn't change the result: the target is a direct ancestor of the source, and doesn't need to be rewound. Add tests to cover the "no rewind required" cases, both when the target's end of WAL is exactly at the point of divergence, which is what the removed assertion assumed (case B in the test), and the case where the assertion failed (case A). Discussion: https://www.postgresql.org/message-id/18575-1b9b3b60e2a480de@postgresql.org --- src/bin/pg_rewind/pg_rewind.c | 7 - src/bin/pg_rewind/t/010_target_is_ancestor.pl | 126 ++++++++++++++++++ 2 files changed, 126 insertions(+), 7 deletions(-) create mode 100644 src/bin/pg_rewind/t/010_target_is_ancestor.pl diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 323c35646c..101e92b549 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -434,16 +434,9 @@ main(int argc, char **argv) * in the target that needs rewinding. */ if (target_wal_endrec > divergerec) - { rewind_needed = true; - } else - { - /* the last common checkpoint record must be part of target WAL */ - Assert(target_wal_endrec == divergerec); - rewind_needed = false; - } } if (!rewind_needed) diff --git a/src/bin/pg_rewind/t/010_target_is_ancestor.pl b/src/bin/pg_rewind/t/010_target_is_ancestor.pl new file mode 100644 index 0000000000..541581a4e8 --- /dev/null +++ b/src/bin/pg_rewind/t/010_target_is_ancestor.pl @@ -0,0 +1,126 @@ + +# Copyright (c) 2021-2024, PostgreSQL Global Development Group + +# Test cases where the target is a direct ancestor of the source. +# +# We set up two timelines like this: +# +# -------------- TLI 2 (source) +# / +# ---A----B-------C------- TLI 1 +# +# +# On TLI 1, we take a backup on points A, B and C. We use a server +# restored from each of those points as the target, and a primary +# server running on TLI 2 as the source. +# +# - A is a direct ancestor of the source, so no rewind is required +# - B is also a direct ancestor of the source, at exactly the point +# where the timelines diverged. No rewind is required. +# - C is not a direct ancestor, so rewind is required. (This case is +# not the focus of these tests, it's here just for completeness.) + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +use File::Copy; +use File::Path qw(rmtree); + +my $tmp_folder = PostgreSQL::Test::Utils::tempdir; + +my $orig_node = PostgreSQL::Test::Cluster->new('orig_node'); +$orig_node->init(allows_streaming => 1); +$orig_node->append_conf( + 'postgresql.conf', qq( +wal_log_hints = on +)); +$orig_node->start; +$orig_node->safe_psql('postgres', 'CREATE TABLE test_tab (t TEXT)'); +$orig_node->safe_psql('postgres', + "INSERT INTO test_tab VALUES ('at the beginning')"); +$orig_node->stop; +$orig_node->backup_fs_cold('backup_A'); + +$orig_node->start; +$orig_node->safe_psql('postgres', + "INSERT INTO test_tab VALUES ('after backup A')"); +$orig_node->stop; +$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->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; + +# Create a new node from a backup, and run pg_rewind to rewind it as a +# follower of $source_node +sub rewind_from_backup +{ + my ($backup, $expected_stderr) = @_; + + my $target_node = PostgreSQL::Test::Cluster->new('restored_' . $backup); + $target_node->init_from_backup($orig_node, $backup, has_streaming => 1); + my $target_pgdata = $target_node->data_dir; + + # Keep a temporary postgresql.conf or it would be overwritten during the rewind. + copy("$target_pgdata/postgresql.conf", "$tmp_folder/postgresql.conf.tmp"); + + command_checks_all( + [ + 'pg_rewind', + "--debug", + "--no-sync", + "--source-pgdata=$source_pgdata", + "--target-pgdata=$target_pgdata" + ], + 0, + [], + [$expected_stderr], + "pg_rewind on node restored from $backup"); + + # Now move back postgresql.conf with old settings + move("$tmp_folder/postgresql.conf.tmp", "$target_pgdata/postgresql.conf"); + + # Configure it to connect to the new primary + my $node_2_connstr = $node_2->connstr(); + $target_node->append_conf( + 'postgresql.conf', qq( +primary_conninfo = '$node_2_connstr' +)); + $target_node->set_standby_mode(); + $target_node->start; + + $node_2->start; + $node_2->wait_for_catchup($target_node->name); + my $result = + $target_node->safe_psql('postgres', "SELECT * FROM test_tab"); + is( $result, qq(at the beginning +after backup A +in node_2), + "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.39.2