From: | Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: multi-install PostgresNode fails with older postgres versions |
Date: | 2021-04-07 05:03:50 |
Message-ID: | E8F512F8-B4D6-4514-BA8D-2E671439DA92@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On Apr 3, 2021, at 11:01 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
> I've had a look at the first of these patches. I think it's generally
> ok, but:
>
>
> - TestLib::system_or_bail('initdb', '-D', $pgdata, '-A', 'trust', '-N',
> + TestLib::system_or_bail('initdb', '-D', $pgdata, '-A', 'trust',
> + $self->at_least_version("9.3") ? '-N' : (),
> @{ $params{extra} });
>
>
> I'd rather do this in two steps to make it clearer.
Changed.
> I still think just doing pg_ctl -w unconditionally would be simpler.
Changed.
> Prior to 9.3 "unix_socket_directories" was spelled
> "unix_socket_directory". We should just set a variable appropriately and
> use it. That should make the changes around that a whole lot simpler.
> (c.f. buildfarm code)
Ah, good to know. Changed.
Other changes:
The v1 patch supported postgres versions back to 8.4, but v2 pushes that back to 8.1.
The version of PostgresNode currently committed relies on IPC::Run in a way that is subtly wrong. The first time IPC::Run::run(X, ...) is called, it uses the PATH as it exists at that time, resolves the path for X, and caches it. Subsequent calls to IPC::Run::run(X, ...) use the cached path, without respecting changes to $ENV{PATH}. In practice, this means that:
use PostgresNode;
my $a = PostgresNode->get_new_node('a', install_path => '/my/install/8.4');
my $b = PostgresNode->get_new_node('b', install_path => '/my/install/9.0');
$a->safe_psql(...) # <=== Resolves and caches 'psql' as /my/install/8.4/bin/psql
$b->safe_psql(...) # <=== Executes /my/install/8.4/bin/psql, not /my/install/9.0/bin/psql as one might expect
PostgresNode::safe_psql() and PostgresNode::psql() both suffer from this, and similarly PostgresNode::pg_recvlogical_upto() because the path to pg_recvlogical gets cached. Calls to initdb and pg_ctl do not appear to suffer this problem, as they are ultimately handled by perl's system() call, not by IPC::Run::run.
Since postgres commands work fairly similarly from one release to another, this can cause subtle and hard to diagnose bugs in regression tests. The fix in v2-0001 works for me, as demonstrated by v2-0002, but whether the fix in the attached v2 patch set gets used or not, I think something needs to be done to fix this.
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Extending-PostgresNode-cross-version-functionalit.patch | application/octet-stream | 15.1 KB |
v2-0002-Adding-modules-test_cross_version.patch | application/octet-stream | 8.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2021-04-07 05:20:50 | Re: Any objections to implementing LogicalDecodeMessageCB for pgoutput? |
Previous Message | Alvaro Herrera | 2021-04-07 03:33:46 | Re: Autovacuum on partitioned table (autoanalyze) |