From ff908940cb278b743389dd9720e8a67295e9cf42 Mon Sep 17 00:00:00 2001 From: Srinath Reddy Sadipiralla Date: Thu, 12 Nov 2026 21:46:08 +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 | 249 ++++++++++++++++-------------- src/bin/pg_rewind/pg_rewind.c | 203 +++++++----------------- src/bin/pg_rewind/pg_rewind.h | 11 +- src/bin/pg_rewind/t/001_basic.pl | 18 +-- src/bin/pg_rewind/t/RewindTest.pm | 19 +-- 5 files changed, 198 insertions(+), 302 deletions(-) diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index 6e87b00f8c2..1172d5bf093 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; @@ -168,9 +111,8 @@ void findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, XLogRecPtr *lastchkptrec, TimeLineID *lastchkpttli, XLogRecPtr *lastchkptredo, uint32 *lastchkptdatachecksums, - const char *restoreCommand) + const char *restoreCommand, XLogRecPtr cntrlfilechkptrec) { - /* Walk backwards, starting from the given record */ XLogRecord *record; XLogRecPtr searchptr; XLogReaderState *xlogreader; @@ -178,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, @@ -201,67 +144,143 @@ 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 (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)); - } + 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; + } - /* Detect if a new WAL file has been opened */ - if (xlogreader->seg.ws_tli != current_tli || - xlogreader->seg.ws_segno != current_segno) + if (!fallback_to_forward) + { + /* We successfully read the first record; proceed with backward scan */ + for (;;) { - char xlogfname[MAXFNAMELEN]; + uint8 info; - snprintf(xlogfname, MAXFNAMELEN, XLOGDIR "/"); + 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)); + } - /* update current values */ - current_tli = xlogreader->seg.ws_tli; - current_segno = xlogreader->seg.ws_segno; + /* 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); + } - XLogFileName(xlogfname + sizeof(XLOGDIR), - current_tli, current_segno, WalSegSz); + /* 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; + + memcpy(&checkPoint, XLogRecGetData(xlogreader), sizeof(CheckPoint)); + *lastchkptrec = searchptr; + *lastchkpttli = checkPoint.ThisTimeLineID; + *lastchkptredo = checkPoint.redo; + *lastchkptdatachecksums = checkPoint.dataChecksumState; + 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 + { /* - * 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. + * Fallback. Scan forward from the control file's last checkpoint. */ - info = XLogRecGetInfo(xlogreader) & ~XLR_INFO_MASK; - if (searchptr < forkptr && - XLogRecGetRmid(xlogreader) == RM_XLOG_ID && - (info == XLOG_CHECKPOINT_SHUTDOWN || - info == XLOG_CHECKPOINT_ONLINE)) + searchptr = cntrlfilechkptrec; + XLogBeginRead(xlogreader, searchptr); + + for (;;) { - CheckPoint checkPoint; + uint8 info; - memcpy(&checkPoint, XLogRecGetData(xlogreader), sizeof(CheckPoint)); - *lastchkptrec = searchptr; - *lastchkpttli = checkPoint.ThisTimeLineID; - *lastchkptredo = checkPoint.redo; - *lastchkptdatachecksums = checkPoint.dataChecksumState; - 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; - /* Walk backwards to previous record. */ - searchptr = record->xl_prev; + /* 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); + } + + /* + * 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; + *lastchkptdatachecksums = checkPoint.dataChecksumState; + } + + /* 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 d2521dab333..6d9cba98881 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 @@ -54,7 +55,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; @@ -107,9 +108,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")); @@ -127,7 +127,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'}, @@ -152,7 +151,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; @@ -217,10 +215,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; @@ -325,30 +319,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); @@ -385,7 +364,6 @@ main(int argc, char **argv) } else { - XLogRecPtr chkptendrec; TimeLineHistoryEntry *sourceHistory; int sourceNentries; @@ -416,47 +394,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) @@ -474,7 +412,7 @@ main(int argc, char **argv) findLastCheckpoint(datadir_target, divergerec, lastcommontliIndex, &chkptrec, &chkpttli, &chkptredo, &chkptdatachecksums, - restore_command); + restore_command, ControlFile_target.checkPoint); pg_log_info("rewinding from last common checkpoint at %X/%08X on timeline %u", LSN_FORMAT_ARGS(chkptrec), chkpttli); @@ -539,8 +477,19 @@ 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 @@ -890,16 +839,6 @@ sanityChecks(void) pg_log_warning_detail("The rewound server will keep data checksums disabled unless the source server enabled them online after the point of divergence."); } - /* - * 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 @@ -1254,79 +1193,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 9187c3bf72f..29d196f73be 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -32,17 +32,16 @@ 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, uint32 *lastchkptdatachecksums, - 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..7154008e1ac 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