Re: Making pg_rewind faster

From: Andres Freund <andres(at)anarazel(dot)de>
To: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
Cc: Justin Kwan <justinpkwan(at)outlook(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, vignesh <vignesh(at)cloudflare(dot)com>, vignesh ravichandran <admin(at)viggy28(dot)dev>, "hlinnaka(at)iki(dot)fi" <hlinnaka(at)iki(dot)fi>, "jkwan(at)cloudflare(dot)com" <jkwan(at)cloudflare(dot)com>
Subject: Re: Making pg_rewind faster
Date: 2022-10-02 17:44:25
Message-ID: 20221002174425.24qgi3ont2g3lvmr@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2022-09-13 20:50:20 +0300, Alexander Korotkov wrote:
> On Fri, Jul 29, 2022 at 1:05 PM Justin Kwan <justinpkwan(at)outlook(dot)com> wrote:
> > Not sure if this email went through previously but thank you for your feedback, I've incorporated your suggestions by scanning the logs produced from pg_rewind when asserting that certain WAL segment files were skipped from being copied over 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(at)anarazel(dot)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

Attachment Content-Type Size
v3-0001-Avoid-copying-WAL-segments-before-divergence-to-s.patch text/x-diff 10.6 KB
v3-0002-Fix-log-file-names-used-in-tests.patch text/x-diff 1.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-10-02 18:05:30 Re: [RFC] building postgres with meson - v13
Previous Message Jonathan S. Katz 2022-10-02 17:32:43 Re: Question: test "aggregates" failed in 32-bit machine