From 2eae57d9e8628bc85a694afba05a3dad0349b9fd Mon Sep 17 00:00:00 2001 From: Srinath Reddy Sadipiralla Date: Sun, 30 Aug 2026 08:48:53 +0530 Subject: [PATCH 1/1] pg_rewind: scan an unclean target's WAL instead of forcing crash recovery Allow pg_rewind to process a stopped target that was not shut down cleanly, without first starting postgres in single-user mode to complete crash recovery. pg_rewind already reads target WAL after the last common checkpoint to construct a page map of target blocks that must be restored from the source. Extend that WAL processing to locate the target WAL endpoint as well, removing the dependency on a shutdown checkpoint or minRecoveryPoint. Normally, find the checkpoint preceding the divergence point by following the WAL record chain backwards. If the record at the divergence point cannot be read, fall back to a forward scan from the target control-file checkpoint, when that checkpoint precedes the divergence point. This avoids replaying target WAL into target relation files just to prepare files whose contents pg_rewind will later restore from the source or remove target files. The target must still be stopped. Replace the clean-shutdown requirement with a check for a running postmaster, and remove a stale postmaster.pid when its PID no longer exists. Remove --no-ensure-shutdown, since pg_rewind no longer starts the target to enforce a clean shutdown. Update pg_rewind tests for the revised target-running behavior and for immediate-stop targets. --- src/bin/pg_rewind/parsexlog.c | 243 ++++++++++++++++-------------- src/bin/pg_rewind/pg_rewind.c | 202 +++++++------------------ src/bin/pg_rewind/pg_rewind.h | 10 +- src/bin/pg_rewind/t/001_basic.pl | 18 +-- src/bin/pg_rewind/t/RewindTest.pm | 19 +-- 5 files changed, 191 insertions(+), 301 deletions(-) diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index 023e23b063c..eee1547c84c 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -56,20 +56,18 @@ 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. + * index 'tliIndex' in target timeline history. Make note of the data blocks + * touched by the WAL records, and return the end of the last valid record. */ -void +XLogRecPtr extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, - XLogRecPtr endpoint, const char *restoreCommand) + const char *restoreCommand) { XLogRecord *record; XLogReaderState *xlogreader; char *errormsg; XLogPageReadPrivate private; + XLogRecPtr endptr; private.tliIndex = tliIndex; private.restoreCommand = restoreCommand; @@ -80,74 +78,19 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, pg_fatal("out of memory while allocating a WAL reading processor"); XLogBeginRead(xlogreader, startpoint); - do + for (;;) { record = XLogReadRecord(xlogreader, &errormsg); if (record == NULL) { - XLogRecPtr errptr = xlogreader->EndRecPtr; - - if (errormsg) - pg_fatal("could not read WAL record at %X/%08X: %s", - LSN_FORMAT_ARGS(errptr), - errormsg); - else + if (errormsg == NULL) pg_fatal("could not read WAL record at %X/%08X", - LSN_FORMAT_ARGS(errptr)); + LSN_FORMAT_ARGS(xlogreader->EndRecPtr)); + break; } extractPageInfo(xlogreader); - } while (xlogreader->EndRecPtr < endpoint); - - /* - * If 'endpoint' didn't point exactly at a record boundary, the caller - * messed up. - */ - if (xlogreader->EndRecPtr != endpoint) - pg_fatal("end pointer %X/%08X is not a valid end point; expected %X/%08X", - LSN_FORMAT_ARGS(endpoint), LSN_FORMAT_ARGS(xlogreader->EndRecPtr)); - - XLogReaderFree(xlogreader); - if (xlogreadfd != -1) - { - close(xlogreadfd); - xlogreadfd = -1; - } -} - -/* - * Reads one WAL record. Returns the end position of the record, without - * doing anything with the record itself. - */ -XLogRecPtr -readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex, - const char *restoreCommand) -{ - XLogRecord *record; - XLogReaderState *xlogreader; - char *errormsg; - XLogPageReadPrivate private; - XLogRecPtr endptr; - - private.tliIndex = tliIndex; - private.restoreCommand = restoreCommand; - xlogreader = XLogReaderAllocate(WalSegSz, datadir, - XL_ROUTINE(.page_read = &SimpleXLogPageRead), - &private); - if (xlogreader == NULL) - pg_fatal("out of memory while allocating a WAL reading processor"); - - XLogBeginRead(xlogreader, ptr); - record = XLogReadRecord(xlogreader, &errormsg); - if (record == NULL) - { - if (errormsg) - pg_fatal("could not read WAL record at %X/%08X: %s", - LSN_FORMAT_ARGS(ptr), errormsg); - else - pg_fatal("could not read WAL record at %X/%08X", - LSN_FORMAT_ARGS(ptr)); } endptr = xlogreader->EndRecPtr; @@ -167,9 +110,9 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex, void findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, XLogRecPtr *lastchkptrec, TimeLineID *lastchkpttli, - XLogRecPtr *lastchkptredo, const char *restoreCommand) + XLogRecPtr *lastchkptredo, const char *restoreCommand, + XLogRecPtr cntrlfilechkptrec) { - /* Walk backwards, starting from the given record */ XLogRecord *record; XLogRecPtr searchptr; XLogReaderState *xlogreader; @@ -177,6 +120,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, XLogPageReadPrivate private; XLogSegNo current_segno = 0; TimeLineID current_tli = 0; + bool fallback_to_forward = false; /* * The given fork pointer points to the end of the last common record, @@ -200,66 +144,137 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, if (xlogreader == NULL) pg_fatal("out of memory while allocating a WAL reading processor"); + /* + * Attempt the standard backward scan first. + */ searchptr = forkptr; - for (;;) - { - uint8 info; + XLogBeginRead(xlogreader, searchptr); + record = XLogReadRecord(xlogreader, &errormsg); - XLogBeginRead(xlogreader, searchptr); - record = XLogReadRecord(xlogreader, &errormsg); + if (record == NULL) + { + /* + * If we fail to read exactly at the forkptr, we assume the target crashed + * exactly at a record boundary and the WAL is padded with zeroes. + * Instead of crashing pg_rewind, we fallback to a forward scan. + */ + fallback_to_forward = true; + } - if (record == NULL) + if (!fallback_to_forward) + { + /* We successfully read the first record; proceed with backward scan */ + for (;;) { - if (errormsg) - pg_fatal("could not find previous WAL record at %X/%08X: %s", - LSN_FORMAT_ARGS(searchptr), - errormsg); - else - pg_fatal("could not find previous WAL record at %X/%08X", - LSN_FORMAT_ARGS(searchptr)); - } + uint8 info; - /* Detect if a new WAL file has been opened */ - if (xlogreader->seg.ws_tli != current_tli || - xlogreader->seg.ws_segno != current_segno) - { - char xlogfname[MAXFNAMELEN]; + if (record == NULL) + { + /* A failure mid-scan means real corruption, so we error out */ + if (errormsg) + pg_fatal("could not find previous WAL record at %X/%08X: %s", + LSN_FORMAT_ARGS(searchptr), errormsg); + else + pg_fatal("could not find previous WAL record at %X/%08X", + LSN_FORMAT_ARGS(searchptr)); + } - snprintf(xlogfname, MAXFNAMELEN, XLOGDIR "/"); + /* Detect if a new WAL file has been opened */ + if (xlogreader->seg.ws_tli != current_tli || + xlogreader->seg.ws_segno != current_segno) + { + char xlogfname[MAXFNAMELEN]; + + snprintf(xlogfname, MAXFNAMELEN, XLOGDIR "/"); + current_tli = xlogreader->seg.ws_tli; + current_segno = xlogreader->seg.ws_segno; + XLogFileName(xlogfname + sizeof(XLOGDIR), + current_tli, current_segno, WalSegSz); + keepwal_add_entry(xlogfname); + } - /* update current values */ - current_tli = xlogreader->seg.ws_tli; - current_segno = xlogreader->seg.ws_segno; + /* Check if it is a valid checkpoint record. */ + info = XLogRecGetInfo(xlogreader) & ~XLR_INFO_MASK; + if (searchptr < forkptr && + XLogRecGetRmid(xlogreader) == RM_XLOG_ID && + (info == XLOG_CHECKPOINT_SHUTDOWN || + info == XLOG_CHECKPOINT_ONLINE)) + { + CheckPoint checkPoint; - XLogFileName(xlogfname + sizeof(XLOGDIR), - current_tli, current_segno, WalSegSz); + memcpy(&checkPoint, XLogRecGetData(xlogreader), sizeof(CheckPoint)); + *lastchkptrec = searchptr; + *lastchkpttli = checkPoint.ThisTimeLineID; + *lastchkptredo = checkPoint.redo; + break; + } - /* Track this filename as one to not remove */ - keepwal_add_entry(xlogfname); + /* Walk backwards to previous record. */ + searchptr = record->xl_prev; + XLogBeginRead(xlogreader, searchptr); + record = XLogReadRecord(xlogreader, &errormsg); } + } + else + { + /* + * Fallback. Scan forward from the control file's last checkpoint. + */ + searchptr = cntrlfilechkptrec; + XLogBeginRead(xlogreader, searchptr); - /* - * Check if it is a checkpoint record. This checkpoint record needs to - * be the latest checkpoint before WAL forked and not the checkpoint - * where the primary has been stopped to be rewound. - */ - info = XLogRecGetInfo(xlogreader) & ~XLR_INFO_MASK; - if (searchptr < forkptr && - XLogRecGetRmid(xlogreader) == RM_XLOG_ID && - (info == XLOG_CHECKPOINT_SHUTDOWN || - info == XLOG_CHECKPOINT_ONLINE)) + for (;;) { - CheckPoint checkPoint; + uint8 info; - memcpy(&checkPoint, XLogRecGetData(xlogreader), sizeof(CheckPoint)); - *lastchkptrec = searchptr; - *lastchkpttli = checkPoint.ThisTimeLineID; - *lastchkptredo = checkPoint.redo; - break; - } + record = XLogReadRecord(xlogreader, &errormsg); + + if (record == NULL) + { + if (errormsg) + pg_fatal("could not read WAL record during forward scan at %X/%08X: %s", + LSN_FORMAT_ARGS(searchptr), errormsg); + else + pg_fatal("could not read WAL record during forward scan at %X/%08X", + LSN_FORMAT_ARGS(searchptr)); + } + + /* Update searchptr to the start of the record we just read */ + searchptr = xlogreader->ReadRecPtr; + + /* Detect if a new WAL file has been opened */ + if (xlogreader->seg.ws_tli != current_tli || + xlogreader->seg.ws_segno != current_segno) + { + char xlogfname[MAXFNAMELEN]; + + snprintf(xlogfname, MAXFNAMELEN, XLOGDIR "/"); + current_tli = xlogreader->seg.ws_tli; + current_segno = xlogreader->seg.ws_segno; + XLogFileName(xlogfname + sizeof(XLOGDIR), + current_tli, current_segno, WalSegSz); + keepwal_add_entry(xlogfname); + } - /* Walk backwards to previous record. */ - searchptr = record->xl_prev; + /* Check if it is a checkpoint record. Update pointers iteratively. */ + info = XLogRecGetInfo(xlogreader) & ~XLR_INFO_MASK; + if (searchptr < forkptr && + XLogRecGetRmid(xlogreader) == RM_XLOG_ID && + (info == XLOG_CHECKPOINT_SHUTDOWN || + info == XLOG_CHECKPOINT_ONLINE)) + { + CheckPoint checkPoint; + + memcpy(&checkPoint, XLogRecGetData(xlogreader), sizeof(CheckPoint)); + *lastchkptrec = searchptr; + *lastchkpttli = checkPoint.ThisTimeLineID; + *lastchkptredo = checkPoint.redo; + } + + /* If we've reached or passed the divergence point, we are done */ + if (xlogreader->EndRecPtr >= forkptr) + break; + } } XLogReaderFree(xlogreader); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 2e86fd158d0..0e77c41b17c 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -9,6 +9,7 @@ */ #include "postgres_fe.h" +#include #include #include #include @@ -53,7 +54,7 @@ static void findCommonAncestorTimeline(TimeLineHistoryEntry *a_history, TimeLineHistoryEntry *b_history, int b_nentries, XLogRecPtr *recptr, int *tliIndex); -static void ensureCleanShutdown(const char *argv0); +static void ensureTargetNotRunning(void); static void disconnect_atexit(void); static ControlFileData ControlFile_target; @@ -106,9 +107,8 @@ usage(const char *progname) printf(_(" -R, --write-recovery-conf write configuration for replication\n" " (requires --source-server)\n")); printf(_(" --config-file=FILENAME use specified main server configuration\n" - " file when running target cluster\n")); + " file when reading target settings\n")); printf(_(" --debug write a lot of debug messages\n")); - printf(_(" --no-ensure-shutdown do not automatically fix unclean shutdown\n")); printf(_(" --sync-method=METHOD set method for syncing files to disk\n")); printf(_(" -V, --version output version information, then exit\n")); printf(_(" -?, --help show this help, then exit\n")); @@ -126,7 +126,6 @@ main(int argc, char **argv) {"write-recovery-conf", no_argument, NULL, 'R'}, {"source-pgdata", required_argument, NULL, 1}, {"source-server", required_argument, NULL, 2}, - {"no-ensure-shutdown", no_argument, NULL, 4}, {"config-file", required_argument, NULL, 5}, {"version", no_argument, NULL, 'V'}, {"restore-target-wal", no_argument, NULL, 'c'}, @@ -150,7 +149,6 @@ main(int argc, char **argv) XLogSegNo last_common_segno; size_t size; char *buffer; - bool no_ensure_shutdown = false; bool rewind_needed; bool writerecoveryconf = false; filemap_t *filemap; @@ -215,10 +213,6 @@ main(int argc, char **argv) connstr_source = pg_strdup(optarg); break; - case 4: - no_ensure_shutdown = true; - break; - case 5: config_file = pg_strdup(optarg); break; @@ -323,30 +317,15 @@ main(int argc, char **argv) else source = init_local_source(datadir_source); - /* - * Check the status of the target instance. - * - * If the target instance was not cleanly shut down, start and stop the - * target cluster once in single-user mode to enforce recovery to finish, - * ensuring that the cluster can be used by pg_rewind. Note that if - * no_ensure_shutdown is specified, pg_rewind ignores this step, and users - * need to make sure by themselves that the target cluster is in a clean - * state. - */ + /* The target must be stopped, but need not have shut down cleanly. */ + ensureTargetNotRunning(); + buffer = slurpFile(datadir_target, XLOG_CONTROL_FILE, &size); digestControlFile(&ControlFile_target, buffer, size); pg_free(buffer); - - if (!no_ensure_shutdown && - ControlFile_target.state != DB_SHUTDOWNED && + if (ControlFile_target.state != DB_SHUTDOWNED && ControlFile_target.state != DB_SHUTDOWNED_IN_RECOVERY) - { - ensureCleanShutdown(argv[0]); - - buffer = slurpFile(datadir_target, XLOG_CONTROL_FILE, &size); - digestControlFile(&ControlFile_target, buffer, size); - pg_free(buffer); - } + pg_log_debug("target was not shut down cleanly; scanning its WAL without recovery"); buffer = source->fetch_file(source, XLOG_CONTROL_FILE, &size); digestControlFile(&ControlFile_source, buffer, size); @@ -383,7 +362,6 @@ main(int argc, char **argv) } else { - XLogRecPtr chkptendrec; TimeLineHistoryEntry *sourceHistory; int sourceNentries; @@ -414,47 +392,7 @@ main(int argc, char **argv) */ pfree(sourceHistory); - - /* - * 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 (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; - } + rewind_needed = true; } if (!rewind_needed) @@ -471,7 +409,7 @@ main(int argc, char **argv) keepwal_init(); findLastCheckpoint(datadir_target, divergerec, lastcommontliIndex, - &chkptrec, &chkpttli, &chkptredo, restore_command); + &chkptrec, &chkpttli, &chkptredo, restore_command, ControlFile_target.checkPoint); pg_log_info("rewinding from last common checkpoint at %X/%08X on timeline %u", LSN_FORMAT_ARGS(chkptrec), chkpttli); @@ -496,8 +434,18 @@ main(int argc, char **argv) */ if (showprogress) pg_log_info("reading WAL in target"); - extractPageMap(datadir_target, chkptrec, lastcommontliIndex, - target_wal_endrec, restore_command); + target_wal_endrec = extractPageMap(datadir_target, chkptrec, + lastcommontliIndex, restore_command); + + /* + * The target WAL must not end before the divergence point. + * If target_wal_endrec == divergerec, the target wrote no WAL past the fork + * point (yielding 0 modified blocks to extract). However, because we already + * verified the timelines diverged, we must still proceed with the file + * synchronization phase to capture timeline history and non-WAL file changes. + */ + if (target_wal_endrec < divergerec) + pg_fatal("target WAL ends before the divergence point"); /* * We have collected all information we need from both systems. Decide @@ -770,16 +718,6 @@ sanityChecks(void) pg_fatal("target server needs to use either data checksums or \"wal_log_hints = on\""); } - /* - * Target cluster better not be running. This doesn't guard against - * someone starting the cluster concurrently. Also, this is probably more - * strict than necessary; it's OK if the target node was not shut down - * cleanly, as long as it isn't running at the moment. - */ - if (ControlFile_target.state != DB_SHUTDOWNED && - ControlFile_target.state != DB_SHUTDOWNED_IN_RECOVERY) - pg_fatal("target server must be shut down cleanly"); - /* * When the source is a data directory, also require that the source * server is shut down. There isn't any very strong reason for this @@ -1134,79 +1072,41 @@ getRestoreCommand(const char *argv0) } -/* - * Ensure clean shutdown of target instance by launching single-user mode - * postgres to do crash recovery. - */ +/* Ensure that the target server is not running. */ static void -ensureCleanShutdown(const char *argv0) +ensureTargetNotRunning(void) { - int ret; - char exec_path[MAXPGPATH]; - PQExpBuffer postgres_cmd; - - /* locate postgres binary */ - if ((ret = find_other_exec(argv0, "postgres", - PG_BACKEND_VERSIONSTR, - exec_path)) < 0) + char pidpath[MAXPGPATH]; + FILE *pidfile; + char line[64]; + long pid; + char *endptr; + + snprintf(pidpath, sizeof(pidpath), "%s/postmaster.pid", datadir_target); + pidfile = fopen(pidpath, "r"); + if (pidfile == NULL) { - char full_path[MAXPGPATH]; - - if (find_my_exec(argv0, full_path) < 0) - strlcpy(full_path, progname, sizeof(full_path)); - - if (ret == -1) - pg_fatal("program \"%s\" is needed by %s but was not found in the same directory as \"%s\"", - "postgres", progname, full_path); - else - pg_fatal("program \"%s\" was found by \"%s\" but was not the same version as %s", - "postgres", full_path, progname); + if (errno == ENOENT) + return; + pg_fatal("could not open file \"%s\" for reading: %m", pidpath); } - - pg_log_info("executing \"%s\" for target server to complete crash recovery", - exec_path); - - /* - * Skip processing if requested, but only after ensuring presence of - * postgres. - */ + if (fgets(line, sizeof(line), pidfile) == NULL || fclose(pidfile) != 0) + pg_fatal("could not read file \"%s\": %m", pidpath); + + errno = 0; + pid = strtol(line, &endptr, 10); + if (errno != 0 || endptr == line || + (*endptr != '\n' && *endptr != '\0') || + pid <= 0 || pid > PG_INT32_MAX) + pg_fatal("invalid PID in file \"%s\"", pidpath); + if (kill((pid_t) pid, 0) == 0 || errno == EPERM) + pg_fatal("target server is running"); + if (errno != ESRCH) + pg_fatal("could not check whether target server is running: %m"); if (dry_run) return; - - /* - * Finally run postgres in single-user mode. There is no need to use - * fsync here. This makes the recovery faster, and the target data folder - * is synced at the end anyway. - */ - postgres_cmd = createPQExpBuffer(); - - /* path to postgres, properly quoted */ - appendShellString(postgres_cmd, exec_path); - - /* add set of options with properly quoted data directory */ - appendPQExpBufferStr(postgres_cmd, " --single -F -D "); - appendShellString(postgres_cmd, datadir_target); - - /* add custom configuration file only if requested */ - if (config_file != NULL) - { - appendPQExpBufferStr(postgres_cmd, " -c config_file="); - appendShellString(postgres_cmd, config_file); - } - - /* finish with the database name, and a properly quoted redirection */ - appendPQExpBufferStr(postgres_cmd, " template1 < "); - appendShellString(postgres_cmd, DEVNULL); - - fflush(NULL); - if (system(postgres_cmd->data) != 0) - { - pg_log_error("postgres single-user mode in target cluster failed"); - pg_log_error_detail("Command was: %s", postgres_cmd->data); - exit(1); - } - - destroyPQExpBuffer(postgres_cmd); + if (unlink(pidpath) != 0) + pg_fatal("could not remove stale file \"%s\": %m", pidpath); } static void diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index 9a981f7f246..b01fd3f0c1d 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -32,16 +32,14 @@ extern uint64 fetch_size; extern uint64 fetch_done; /* in parsexlog.c */ -extern void extractPageMap(const char *datadir, XLogRecPtr startpoint, - int tliIndex, XLogRecPtr endpoint, - const char *restoreCommand); +extern XLogRecPtr extractPageMap(const char *datadir, XLogRecPtr startpoint, + int tliIndex, + const char *restoreCommand); extern void findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, XLogRecPtr *lastchkptrec, TimeLineID *lastchkpttli, XLogRecPtr *lastchkptredo, - const char *restoreCommand); -extern XLogRecPtr readOneRecord(const char *datadir, XLogRecPtr ptr, - int tliIndex, const char *restoreCommand); + const char *restoreCommand, XLogRecPtr cntrlfilechkptrec); /* in pg_rewind.c */ extern void progress_report(bool finished); diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl index 8d6ab3484b8..5d54177a8e4 100644 --- a/src/bin/pg_rewind/t/001_basic.pl +++ b/src/bin/pg_rewind/t/001_basic.pl @@ -100,9 +100,7 @@ sub run_test my $primary_pgdata = $node_primary->data_dir; my $standby_pgdata = $node_standby->data_dir; - # First check that pg_rewind fails if the target cluster is - # not stopped as it fails to start up for the forced recovery - # step. + # First check that pg_rewind refuses a running target cluster. command_fails( [ 'pg_rewind', '--debug', @@ -112,18 +110,6 @@ sub run_test ], 'pg_rewind with running target'); - # Again with --no-ensure-shutdown, which should equally fail. - # This time pg_rewind complains without attempting to perform - # recovery once. - command_fails( - [ - 'pg_rewind', '--debug', - '--source-pgdata' => $standby_pgdata, - '--target-pgdata' => $primary_pgdata, - '--no-sync', '--no-ensure-shutdown' - ], - 'pg_rewind --no-ensure-shutdown with running target'); - # Stop the target, and attempt to run with a local source # still running. This fails as pg_rewind requires to have # a source cleanly stopped. @@ -133,7 +119,7 @@ sub run_test 'pg_rewind', '--debug', '--source-pgdata' => $standby_pgdata, '--target-pgdata' => $primary_pgdata, - '--no-sync', '--no-ensure-shutdown' + '--no-sync' ], 'pg_rewind with unexpected running source'); diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm index 32aeca80f13..0df67acc7e6 100644 --- a/src/bin/pg_rewind/t/RewindTest.pm +++ b/src/bin/pg_rewind/t/RewindTest.pm @@ -219,21 +219,14 @@ sub run_pg_rewind if ($test_mode eq 'archive') { # pg_rewind is tested with --restore-target-wal by moving all - # WAL files to a secondary location. Note that this leads to - # a failure in ensureCleanShutdown(), forcing to the use of - # --no-ensure-shutdown in this mode as the initial set of WAL - # files needed to ensure a clean restart is gone. This could - # be improved by keeping around only a minimum set of WAL - # segments but that would just make the test more costly, - # without improving the coverage. Hence, instead, stop - # gracefully the primary here. + # WAL files to a secondary location. Stop gracefully here because the + # target's final WAL record must remain available locally. $node_primary->stop; } else { - # Stop the primary and be ready to perform the rewind. The cluster - # needs recovery to finish once, and pg_rewind makes sure that it - # happens automatically. + # Stop the primary and be ready to perform the rewind. pg_rewind scans + # its WAL to locate the final valid record without performing recovery. $node_primary->stop('immediate'); } @@ -329,8 +322,7 @@ sub run_pg_rewind # Stop the new primary and be ready to perform the rewind. $node_standby->stop; - # Note the use of --no-ensure-shutdown here. WAL files are - # gone in this mode and the primary has been stopped + # WAL files are gone in this mode and the primary has been stopped # gracefully already. --config-file reuses the original # postgresql.conf as restore_command has been enabled above. command_ok( @@ -340,7 +332,6 @@ sub run_pg_rewind '--source-pgdata' => $standby_pgdata, '--target-pgdata' => $primary_pgdata, '--no-sync', - '--no-ensure-shutdown', '--restore-target-wal', '--config-file' => "$primary_pgdata/postgresql.conf", ], -- 2.43.0