Re: Executing pg_createsubscriber with a non-compatible control file

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Euler Taveira <euler(at)eulerto(dot)com>
Subject: Re: Executing pg_createsubscriber with a non-compatible control file
Date: 2025-10-14 08:04:12
Message-ID: aO4D_NKBWWOmaJnE@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 13, 2025 at 03:35:06PM -0700, Masahiko Sawada wrote:
> v1-0001-Introduce-API-able-to-retrieve-contents-of-PG_VER.patch:
>
> v1-0002-pg_upgrade-Use-PG_VERSION-generic-routine.patch:
> v1-0003-pg_createsubscriber-Use-PG_VERSION-generic-routin.patch:

Applied both of these.

> The new log detail message uses the same message as what pg_resetwal
> uses, but pg_createsubscriber shows an integer major version whereas
> pg_resetwal shows the raw version string. I guess it's better to unify
> the usage for better consistency.

OK, done as suggested to limit the translation work.

> v1-0004-pg_combinebackup-use-PG_VERSION-generic-routine.patch:
>
> + pg_log_debug("read server version %u from file \"%s\"",
> + GET_PG_MAJORVERSION_NUM(version), "PG_VERSION");
>
> We used to show the full path of PG_VERSION file in the debug message.
> While probably we can live with such a small difference, do you think
> it's a good idea to move the debug message to get_pg_version()?

I cannot get into doing that, leaving that up to the caller with the
error message they want. That's a minor point, I guess, I can see
both sides of the coin.

Switched this one to report the full path, like previously.

> v1-0005-pg_resetwal-use-PG_VERSION-generic-routine.patch:
>
> /* Check that data directory matches our server version */
> - CheckDataVersion();
> + CheckDataVersion(DataDir);
>
> With the patch, pg_resetwal fails to handle a relative path of PGDATA
> as follows:
>
> $ bin/pg_resetwal data
> pg_resetwal: error: could not open version file "data/PG_VERSION": No
> such file or directory
>
> This is because pg_resetwal does chdir() to the given path of the data
> directory before checking the version.

Right. I've tested with absolute paths and forgot relative paths.
For this one, I would just use a ".", as the chdir is before the
version check. Or we could reverse the chdir() and the version check,
but there is no real benefit in doing so.

Updated patch set attached.
--
Michael

Attachment Content-Type Size
v2-0001-pg_createsubscriber-Use-PG_VERSION-generic-routin.patch text/x-diff 1.9 KB
v2-0002-pg_combinebackup-use-PG_VERSION-generic-routine.patch text/x-diff 3.8 KB
v2-0003-pg_resetwal-use-PG_VERSION-generic-routine.patch text/x-diff 1.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Richard Guo 2025-10-14 08:04:53 Re: A tidyup of pathkeys.c
Previous Message Michael Paquier 2025-10-14 08:01:14 Re: Improved TAP tests by replacing sub-optimal uses of ok() with better Test::More functions