Hi,
On 2022-09-13 20:50:20 +0300, Alexander Korotkov wrote:
> On Fri, Jul 29, 2022 at 1:05 PM Justin Kwan <justinpkwan@outlook.com> wrote:
> > Not sure if this email went through previously but thank you for your feedback, I've incorporated your suggestions
byscanning the logs produced from pg_rewind when asserting that certain WAL segment files were skipped from being
copiedover to the target server.
> >
> > I've also updated the pg_rewind patch file to target the Postgres master branch (version 16 to be). Please see
attached.
>
> Thank you for the revision.
>
> I've taken a look at this patch. Overall it looks good to me. I also
> don't see any design objections in the thread.
>
> A couple of points from me:
> 1) I would prefer to evade hard-coded names for WAL segments in the
> tap tests. Could we calculate the names in the tap tests based on the
> diverge point, etc.?
> 2) Patch contains some indentation with spaces, which should be done
> in tabs. Please consider either manually fixing this or running
> pgindent over modified files.
This patch currently fails because it hasn't been adjusted for
commit c47885bd8b6
Author: Andres Freund <andres@anarazel.de>
Date: 2022-09-19 18:03:17 -0700
Split TESTDIR into TESTLOGDIR and TESTDATADIR
The adjustment is trivial. Attached, together with also producing an error
message rather than just dying wordlessly.
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.
With regard to Alexander's point about whitespace:
.git/rebase-apply/patch:25: indent with spaces.
/* Handle WAL segment file. */
.git/rebase-apply/patch:26: indent with spaces.
const char *fname;
.git/rebase-apply/patch:27: indent with spaces.
char *slash;
.git/rebase-apply/patch:29: indent with spaces.
/* Split filepath into directory & filename. */
.git/rebase-apply/patch:30: indent with spaces.
slash = strrchr(path, '/');
warning: squelched 29 whitespace errors
warning: 34 lines add whitespace errors.
Greetings,
Andres Freund
