From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
Cc: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pg_upgrade version checking questions |
Date: | 2021-02-23 16:14:28 |
Message-ID: | 22906594-DDD7-4085-8318-595C0C9C75C6@yesql.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 27 Jul 2019, at 08:42, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>
> On 2019-07-25 16:33, Daniel Gustafsson wrote:
>> Fair enough, those are both excellent points. I’ve shuffled the code around to
>> move back the check for exec_path to setup (albeit earlier than before due to
>> where we perform directory checking). This does mean that the directory
>> checking in the options parsing must learn to cope with missing directories,
>> which is a bit unfortunate since it’s already doing a few too many things IMHO.
>> To ensure dogfooding, I also removed the use of -B in ‘make check’ for
>> pg_upgrade, which should bump the coverage.
>>
>> Also spotted a typo in a pg_upgrade file header in a file touched by this, so
>> included it in this thread too as a 0004.
>
> I have committed 0002, 0003, and 0004.
>
> The implementation in 0001 (Only allow upgrades by the same exact
> version new bindir) has a problem. It compares (new_cluster.bin_version
> != PG_VERSION_NUM), but new_cluster.bin_version is actually just the
> version of pg_ctl, so this is just comparing the version of pg_upgrade
> with the version of pg_ctl, which is not wrong, but doesn't really
> achieve the full goal of having all binaries match.
>
> I think a better structure would be to add a version check for each
> validate_exec() so that each program is checked against pg_upgrade.
> This should mirror what find_other_exec() in src/common/exec.c does. In
> a better world we would use find_other_exec() directly, but then we
> can't support -B. Maybe expand find_other_exec() to support this, or
> make a private copy for pg_upgrade to support this. (Also, we have two
> copies of validate_exec() around. Maybe this could all be unified.)
Turns out I overshot my original estimate of a new 0001 by a hair (by ~530 days
or so) but attached is an updated version.
This exports validate_exec to reduce duplication, and implements a custom
find_other_exec-like function in pg_upgrade to check each binary for the
version number. Keeping a local copy of validate_exec is easy to do if it's
deemed not worth it to export it.
--
Daniel Gustafsson https://vmware.com/
Attachment | Content-Type | Size |
---|---|---|
v5-0001-Check-version-of-target-cluster-binaries.patch | application/octet-stream | 5.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Konstantin Knizhnik | 2021-02-23 16:29:12 | Re: libpq compression |
Previous Message | Justin Pryzby | 2021-02-23 16:03:18 | Re: [PATCH]: Allow errors in parameter values to be reported during the BIND phase itself.. |