Re: PATCH: add "--config-file=" option to pg_rewind

From: "Gunnar \"Nick\" Bluth" <gunnar(dot)bluth(at)pro-open(dot)de>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Alexander Kukushkin <cyberdemn(at)gmail(dot)com>, Aleksander Alekseev <aleksander(at)timescale(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, x4mmm(at)yandex-team(dot)ru, kondratovaleksey(at)gmail(dot)com
Subject: Re: PATCH: add "--config-file=" option to pg_rewind
Date: 2022-02-26 08:55:20
Message-ID: 635d61d3-a1de-95e5-50fb-7225066ff12f@pro-open.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Am 26.02.22 um 06:51 schrieb Michael Paquier:
> On Fri, Feb 25, 2022 at 10:35:49AM +0100, Gunnar "Nick" Bluth wrote:
>> Am 24.02.22 um 14:46 schrieb Daniel Gustafsson:
>>> Actually, I think this looks like a saner approach. Putting a config setting
>>> in two place (postgresql.conf and on the commandline for pg_rewind) is a recipe
>>> for them diverging.
>
> FWIW, I have a bad feeling about passing down directly a command
> through an option itself part of a command, so what's discussed on
> this thread is refreshing.

Ta.

>
> + } else {
> + snprintf(postgres_cmd, sizeof(postgres_cmd),
> + "\"%s\" -D \"%s\" --config_file=\"%s\" -C restore_command",
> + postgres_exec_path, datadir_target, config_file);
> + }
>
> Shouldn't this one use appendShellString() on config_file?

It probably should, yes. I don't fancy this repetitive code myself.
But then, pg_rewind as a whole could use an overhaul. I don't see any
use of PQExpBuffer in it, but a lot of char* ...

I myself am not a C hacker (as you can probably tell already) and don't
really feel fit (enough) to rewrite pg_rewind.c to use
fe_utils/string_utils.h :/

I might give it a try anyhow, but that may be a bit... heavy,,, just to
add one option?

While we're at it (I'm really good at opening cans of worms):
09:47 $ grep -r -l --include \*.c fe_utils/string_utils.h src/bin/ | wc -l
28
09:48 $ grep -r -L --include \*.c fe_utils/string_utils.h src/bin/ | wc -l
66

GSOC project? ;-)

Best,
--
Gunnar "Nick" Bluth

Eimermacherweg 106
D-48159 Münster

Mobil +49 172 8853339
Email: gunnar(dot)bluth(at)pro-open(dot)de
__________________________________________________________________________
"Ceterum censeo SystemD esse delendam" - Cato

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2022-02-26 09:16:56 Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers)
Previous Message Bharath Rupireddy 2022-02-26 08:47:50 Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers)