On Sun, Oct 02, 2022 at 10:44:25AM -0700, Andres Freund wrote:
> It doesn't seem quite right to read pg_rewind's logs by reading
> regress_log_001_basic. Too easy to confuse different runs of pg_rewind
> etc. I'd suggest trying to redirect the log to a different file.
Hardcoding log file names in the test increases the overall
maintenance, even if renaming these would be easy to track and fix if
the naming convention is changed. Anyway, I think that what this
patch should do is to use command_checks_all() in RewindTest.pm as it
is the test API able to check after a status and multiple expected
outputs, which is what the changes in 001 and 008 are doing.
RewindTest::run_pg_rewind() needs to be a bit tweaked to accept these
regex patterns in input.
+ if (file_segno < last_common_segno)
+ {
+ pg_log_debug("WAL file entry \"%s\" not copied to target", fname);
+ return FILE_ACTION_NONE;
+ }
There may be something I am missing here, but there is no need to care
about segments with a TLI older than lastcommontliIndex, no?
decide_wal_file_action() assumes that the WAL segment exists on the
target and the source. This looks bug-prone to me without at least an
assertion.
file_entry_t has an entry to track if a file is a relation file. I
think that it would be much cleaner to track if we are handling a WAL
segment when inserting an entry in insert_filehash_entry(), so
isrelfile could be replaced by an enum with three values: relation
file, WAL segment and the rest.
--
Michael