From 47ae21af8fd2d685987b10c3195e4897c1f81976 Mon Sep 17 00:00:00 2001 From: jcoleman Date: Fri, 3 Jun 2022 20:32:15 -0400 Subject: [PATCH v1] pg_rewind: warn if we haven't checkpointed after promote Because pg_rewind looks at the last checkpoint to determine the timeline on the source server it can incorrectly report that the timelines are the same if the source is a recently promoted standby that hasn't yet completed a new checkpoint. This is already documented, but we shouldn't tell the user a rewind isn't necessary when it clearly is. --- src/bin/pg_rewind/pg_rewind.c | 13 +++- .../t/010_no_checkpoint_after_promotion.pl | 74 +++++++++++++++++++ src/bin/pg_rewind/t/RewindTest.pm | 22 ++++-- 3 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 src/bin/pg_rewind/t/010_no_checkpoint_after_promotion.pl diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 1ff8da1676..2dc396c4fd 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -337,8 +337,19 @@ main(int argc, char **argv) * * If both clusters are already on the same timeline, there's nothing to * do. + * + * We must avoid assuming that a checkpoint has happened on the source or + * we'll possibly conclude the timelines are the same when the source has + * recently promoted. */ - if (ControlFile_target.checkPointCopy.ThisTimeLineID == + if (ControlFile_source.minRecoveryPointTLI != 0 && + ControlFile_source.checkPointCopy.ThisTimeLineID != + ControlFile_source.minRecoveryPointTLI) + { + pg_log_info("source cluster hasn't checkpointed since its last timeline switch"); + exit(1); + } + else if (ControlFile_target.checkPointCopy.ThisTimeLineID == ControlFile_source.checkPointCopy.ThisTimeLineID) { pg_log_info("source and target cluster are on the same timeline"); diff --git a/src/bin/pg_rewind/t/010_no_checkpoint_after_promotion.pl b/src/bin/pg_rewind/t/010_no_checkpoint_after_promotion.pl new file mode 100644 index 0000000000..53188b2514 --- /dev/null +++ b/src/bin/pg_rewind/t/010_no_checkpoint_after_promotion.pl @@ -0,0 +1,74 @@ + +# Copyright (c) 2021-2022, PostgreSQL Global Development Group + +use strict; +use warnings; +use PostgreSQL::Test::Utils; +use Test::More; + +use FindBin; +use lib $FindBin::RealBin; + +use RewindTest; + +RewindTest::setup_cluster("remote"); +RewindTest::start_primary(); + +# Create a test table and insert a row in primary. +primary_psql("CREATE TABLE tbl1 (d text)"); +primary_psql("INSERT INTO tbl1 VALUES ('in primary')"); + +# This test table will be used to test truncation, i.e. the table +# is extended in the old primary after promotion +primary_psql("CREATE TABLE trunc_tbl (d text)"); +primary_psql("INSERT INTO trunc_tbl VALUES ('in primary')"); + +# This test table will be used to test the "copy-tail" case, i.e. the +# table is truncated in the old primary after promotion +primary_psql("CREATE TABLE tail_tbl (id integer, d text)"); +primary_psql("INSERT INTO tail_tbl VALUES (0, 'in primary')"); + +# This test table is dropped in the old primary after promotion. +primary_psql("CREATE TABLE drop_tbl (d text)"); +primary_psql("INSERT INTO drop_tbl VALUES ('in primary')"); + +primary_psql("CHECKPOINT"); + +RewindTest::create_standby("remote"); + +# Insert additional data on primary that will be replicated to standby +primary_psql("INSERT INTO tbl1 values ('in primary, before promotion')"); +primary_psql( + "INSERT INTO trunc_tbl values ('in primary, before promotion')"); +primary_psql( + "INSERT INTO tail_tbl SELECT g, 'in primary, before promotion: ' || g FROM generate_series(1, 10000) g" +); + +primary_psql('CHECKPOINT'); + +RewindTest::promote_standby('skip_checkpoint' => 1); + +# Insert a row in the old primary. This causes the primary and standby +# to have "diverged", it's no longer possible to just apply the +# standy's logs over primary directory - you need to rewind. +primary_psql("INSERT INTO tbl1 VALUES ('in primary, after promotion')"); + +# Also insert a new row in the standby, which won't be present in the +# old primary. +standby_psql("INSERT INTO tbl1 VALUES ('in standby, after promotion')"); + +$node_primary->stop; + +my $primary_pgdata = $node_primary->data_dir; +my $standby_connstr = $node_standby->connstr('postgres'); +command_fails_like( + [ + 'pg_rewind', '--dry-run', + "--source-server", $standby_connstr, + '--target-pgdata', $primary_pgdata, + '--no-sync', '--no-ensure-shutdown' + ], + qr/source cluster hasn\'t checkpointed since its last timeline switch/, + 'pg_rewind no checkpoint after promotion'); + +done_testing(); diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm index 98b66b01f8..89fddc1656 100644 --- a/src/bin/pg_rewind/t/RewindTest.pm +++ b/src/bin/pg_rewind/t/RewindTest.pm @@ -188,6 +188,11 @@ primary_conninfo='$connstr_primary' sub promote_standby { + my (%options) = ( + 'skip_checkpoint' => 0, + @_ + ); + #### Now run the test-specific parts to run after standby has been started # up standby @@ -198,13 +203,16 @@ sub promote_standby # the primary out-of-sync with the standby. $node_standby->promote; - # Force a checkpoint after the promotion. pg_rewind looks at the control - # file to determine what timeline the server is on, and that isn't updated - # immediately at promotion, but only at the next checkpoint. When running - # pg_rewind in remote mode, it's possible that we complete the test steps - # after promotion so quickly that when pg_rewind runs, the standby has not - # performed a checkpoint after promotion yet. - standby_psql("checkpoint"); + unless ($options{'skip_checkpoint'}) + { + # Force a checkpoint after the promotion. pg_rewind looks at the control + # file to determine what timeline the server is on, and that isn't updated + # immediately at promotion, but only at the next checkpoint. When running + # pg_rewind in remote mode, it's possible that we complete the test steps + # after promotion so quickly that when pg_rewind runs, the standby has not + # performed a checkpoint after promotion yet. + standby_psql("checkpoint"); + } return; } -- 2.32.0 (Apple Git-132)