public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v2 2/2] Correctly scan all WAL in target, if it was shut down in recovery. 3+ messages / 2 participants [nested] [flat]
* [PATCH v2 2/2] Correctly scan all WAL in target, if it was shut down in recovery. @ 2020-12-02 11:04 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Heikki Linnakangas @ 2020-12-02 11:04 UTC (permalink / raw) If minRecoveryPoint is beyond the last checkpoint, we need to scan the WAL all the way up to minRecoveryPoint. --- src/bin/pg_rewind/parsexlog.c | 11 ++- src/bin/pg_rewind/pg_rewind.c | 63 ++++++------ src/bin/pg_rewind/t/000_min_recovery_point.pl | 98 ++++++++++++++----- 3 files changed, 120 insertions(+), 52 deletions(-) diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index eae1797f94..9275cba51b 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -55,6 +55,9 @@ static int SimpleXLogPageRead(XLogReaderState *xlogreader, * Read WAL from the datadir/pg_wal, starting from 'startpoint' on timeline * index 'tliIndex' in target timeline history, until 'endpoint'. Make note of * the data blocks touched by the WAL records, and return them in a page map. + * + * 'endpoint' is the end of the last record to read. The record starting at + * 'endpoint' is the first one that is not read. */ void extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, @@ -93,7 +96,13 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, extractPageInfo(xlogreader); - } while (xlogreader->ReadRecPtr != endpoint); + } while (xlogreader->EndRecPtr < endpoint); + + /* + * If 'endpoint' didn't point exactly at a record boundary, the caller + * messed up. + */ + Assert(xlogreader->EndRecPtr == endpoint); XLogReaderFree(xlogreader); if (xlogreadfd != -1) diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index f2f815042f..9791ebac72 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -129,6 +129,7 @@ main(int argc, char **argv) XLogRecPtr chkptrec; TimeLineID chkpttli; XLogRecPtr chkptredo; + XLogRecPtr target_wal_endrec; size_t size; char *buffer; bool no_ensure_shutdown = false; @@ -335,50 +336,56 @@ main(int argc, char **argv) { pg_log_info("source and target cluster are on the same timeline"); rewind_needed = false; + target_wal_endrec = 0; } else { + XLogRecPtr chkptendrec; + findCommonAncestorTimeline(&divergerec, &lastcommontliIndex); pg_log_info("servers diverged at WAL location %X/%X on timeline %u", (uint32) (divergerec >> 32), (uint32) divergerec, targetHistory[lastcommontliIndex].tli); + /* + * Determine the end-of-WAL on the target. + * + * The WAL ends at the last shutdown checkpoint, or at minRecoveryPoint if + * it was a standby. (If we supported rewinding a server that was not shut + * down cleanly, we would need to replay until we reach the first invalid + * record, like crash recovery does.) + */ + + /* read the checkpoint record on the target to see where it ends. */ + chkptendrec = readOneRecord(datadir_target, + ControlFile_target.checkPoint, + targetNentries - 1, + restore_command); + + if (ControlFile_target.minRecoveryPoint > chkptendrec) + { + target_wal_endrec = ControlFile_target.minRecoveryPoint; + } + else + { + target_wal_endrec = chkptendrec; + } + /* * Check for the possibility that the target is in fact a direct * ancestor of the source. In that case, there is no divergent history * in the target that needs rewinding. */ - if (ControlFile_target.checkPoint >= divergerec) + if (target_wal_endrec > divergerec) { rewind_needed = true; } else { - XLogRecPtr chkptendrec; - - /* Read the checkpoint record on the target to see where it ends. */ - chkptendrec = readOneRecord(datadir_target, - ControlFile_target.checkPoint, - targetNentries - 1, - restore_command); + /* the last common checkpoint record must be part of the target WAL */ + Assert(target_wal_endrec == divergerec); - /* - * If the minimum recovery ending location is beyond the end of - * the last checkpoint, that means there are WAL records beyond - * the divergence point and a rewind is needed. - */ - if (ControlFile_target.minRecoveryPoint > chkptendrec) - rewind_needed = true; - /* - * If the histories diverged exactly at the end of the shutdown - * checkpoint record on the target, there are no WAL records in - * the target that don't belong in the source's history, and no - * rewind is needed. - */ - else if (chkptendrec == divergerec) - rewind_needed = false; - else - rewind_needed = true; + rewind_needed = false; } } @@ -414,14 +421,12 @@ main(int argc, char **argv) /* * Read the target WAL from last checkpoint before the point of fork, to * extract all the pages that were modified on the target cluster after - * the fork. We can stop reading after reaching the final shutdown record. - * XXX: If we supported rewinding a server that was not shut down cleanly, - * we would need to replay until the end of WAL here. + * the fork. */ if (showprogress) pg_log_info("reading WAL in target"); extractPageMap(datadir_target, chkptrec, lastcommontliIndex, - ControlFile_target.checkPoint, restore_command); + target_wal_endrec, restore_command); /* * We have collected all information we need from both systems. Decide diff --git a/src/bin/pg_rewind/t/000_min_recovery_point.pl b/src/bin/pg_rewind/t/000_min_recovery_point.pl index ac842fd0ed..84b0e947c1 100644 --- a/src/bin/pg_rewind/t/000_min_recovery_point.pl +++ b/src/bin/pg_rewind/t/000_min_recovery_point.pl @@ -1,7 +1,28 @@ # # Test situation where a target data directory contains # WAL records beyond both the last checkpoint and the divergence -# point. +# point: +# +# Target WAL (TLI 2): +# +# backup ... Checkpoint A ... INSERT 'rewind this' +# (TLI 1 -> 2) +# +# ^ last common ^ minRecoveryPoint +# checkpoint +# +# Source WAL (TLI 3): +# +# backup ... Checkpoint A ... Checkpoint B ... INSERT 'keep this' +# (TLI 1 -> 2) (TLI 2 -> 3) +# +# +# The last common checkpoint is Checkpoint A. But there is WAL on TLI 2 +# after the last common checkpoint that needs to be rewound. We used to +# have a bug where minRecoveryPoint was ignored, and pg_rewind concluded +# that the target doesn't need rewinding in this scenario, because the +# last checkpoint on the target TLI was an ancestor of the source TLI. +# # # This test does not make use of RewindTest as it requires three # nodes. @@ -10,20 +31,26 @@ use strict; use warnings; use PostgresNode; use TestLib; -use Test::More tests => 2; +use Test::More tests => 3; + +use File::Copy; + +my $tmp_folder = TestLib::tempdir; my $node_1 = get_new_node('node_1'); $node_1->init(allows_streaming => 1); $node_1->append_conf( 'postgresql.conf', qq( -wal_keep_size=16 +wal_keep_size='100 MB' )); $node_1->start; -# Add an arbitrary table -$node_1->safe_psql('postgres', - 'CREATE TABLE public.foo (id INT)'); +# Create a couple of test tables +$node_1->safe_psql('postgres', 'CREATE TABLE public.foo (t TEXT)'); +$node_1->safe_psql('postgres', 'CREATE TABLE public.bar (t TEXT)'); +$node_1->safe_psql('postgres', "INSERT INTO public.bar VALUES ('in both')"); + # Take backup my $backup_name = 'my_backup'; @@ -84,35 +111,62 @@ $node_2->restart(); $node_1->promote; # We now have a split-brain with two primaries. Insert a row on both to -# demonstratively create a split brain; this is not strictly necessary -# for this test, but creates an easily identifiable WAL record and -# enables us to verify that node_2 has the required changes to -# reproduce the situation we're handling. - -$node_1->safe_psql('postgres', 'INSERT INTO public.foo (id) VALUES (0)'); -$node_3->safe_psql('postgres', 'INSERT INTO public.foo (id) VALUES (1)'); - +# demonstratively create a split brain. After the rewind, we should only +# see the insert on 1, as the insert on node 3 is rewound away. +$node_1->safe_psql('postgres', "INSERT INTO public.foo (t) VALUES ('keep this')"); + +# Insert more rows in node 1, to bump up the XID counter. Otherwise, if +# rewind doesn't correctly rewind the changes made on the other node, +# we might fail to notice if the inserts are invisible because the XIDs +# are not marked as committed. +$node_1->safe_psql('postgres', "INSERT INTO public.foo (t) VALUES ('and this')"); +$node_1->safe_psql('postgres', "INSERT INTO public.foo (t) VALUES ('and this too')"); + +# Also insert a row in 'bar' on node 3. It is unmodified in node 1, so it won't get +# overwritten by replaying the WAL from node 1. +$node_3->safe_psql('postgres', "INSERT INTO public.bar (t) VALUES ('rewind this')"); + +# Wait for node 2 to catch up $node_2->poll_query_until('postgres', - q|SELECT COUNT(*) > 0 FROM public.foo|, 't'); + q|SELECT COUNT(*) > 1 FROM public.bar|, 't'); # At this point node_2 will shut down without a shutdown checkpoint, # but with WAL entries beyond the preceding shutdown checkpoint. $node_2->stop('fast'); $node_3->stop('fast'); - my $node_2_pgdata = $node_2->data_dir; my $node_1_connstr = $node_1->connstr; -command_checks_all( +# Keep a temporary postgresql.conf or it would be overwritten during the rewind. +copy( + "$node_2_pgdata/postgresql.conf", + "$tmp_folder/node_2-postgresql.conf.tmp"); + +command_ok( [ 'pg_rewind', "--source-server=$node_1_connstr", "--target-pgdata=$node_2_pgdata", - '--dry-run' + '--write-recovery-conf' ], - 0, - [], - [qr|rewinding from last common checkpoint at|], - 'pg_rewind detects rewind needed'); + 'pg_rewind detects rewind needed'); + +# Now move back postgresql.conf with old settings +move( + "$tmp_folder/node_2-postgresql.conf.tmp", + "$node_2_pgdata/postgresql.conf"); + +$node_2->start; +# Check contents of the test tables after rewind. The rows inserted in node 3 +# before rewind should've been overwritten with the data from node 1. +my $result; +$result = $node_2->safe_psql('postgres', 'checkpoint'); +$result = $node_2->safe_psql('postgres', 'SELECT * FROM public.foo'); +is($result, qq(keep this +and this +and this too), 'table foo after rewind'); + +$result = $node_2->safe_psql('postgres', 'SELECT * FROM public.bar'); +is($result, qq(in both), 'table bar after rewind'); -- 2.20.1 --------------0E43093204FD259F5575BE74-- ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: RFC: Extension Packaging & Lookup @ 2024-10-29 17:03 David E. Wheeler <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: David E. Wheeler @ 2024-10-29 17:03 UTC (permalink / raw) To: Christoph Berg <[email protected]>; +Cc: Paul Ramsey <[email protected]>; pgsql-hackers; Gabriele Bartolini <[email protected]>; Peter Eisentraut <[email protected]>; Andres Freund <[email protected]> On Oct 29, 2024, at 12:51, Christoph Berg <[email protected]> wrote: > I think this is where the whole idea of "provide binaries outside of > deb/rpm" is just going to die. You are trying to reinvent a wheel that > has been running well for decades, including lots of production > systems. I don't know anyone who would trust that new source of > binaries that doesn't integrate into their OS packaging system. That’s fine for Linux, but more challenging for macOS and Windows. It’s also an issue that the apt and yum repositories, while having a lot of stuff, don’t have all extensions. D ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: RFC: Extension Packaging & Lookup @ 2024-10-29 17:11 David E. Wheeler <[email protected]> parent: David E. Wheeler <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: David E. Wheeler @ 2024-10-29 17:11 UTC (permalink / raw) To: Christoph Berg <[email protected]>; +Cc: Paul Ramsey <[email protected]>; pgsql-hackers; Gabriele Bartolini <[email protected]>; Peter Eisentraut <[email protected]>; Andres Freund <[email protected]> On Oct 29, 2024, at 13:03, David E. Wheeler <[email protected]> wrote: > That’s fine for Linux, but more challenging for macOS and Windows. It’s also an issue that the apt and yum repositories, while having a lot of stuff, don’t have all extensions. Sorry, I think I was too quick to respond there. To the degree possible, it makes sense to me that one would try to depend on DSO packages provided by OS packaging systems. For non-Linux systems, that might mean requiring a third-party packaging system like Homebrew (macOS) or Chocolatey (Windows). The trick will be how to add such dependencies to a Docker container at runtime. Best, David ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-10-29 17:11 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-12-02 11:04 [PATCH v2 2/2] Correctly scan all WAL in target, if it was shut down in recovery. Heikki Linnakangas <[email protected]> 2024-10-29 17:03 Re: RFC: Extension Packaging & Lookup David E. Wheeler <[email protected]> 2024-10-29 17:11 ` Re: RFC: Extension Packaging & Lookup David E. Wheeler <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox