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 |
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 |