From: | "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com> |
---|---|
To: | vignesh C <vignesh21(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Add two missing tests in 035_standby_logical_decoding.pl |
Date: | 2023-04-26 08:14:18 |
Message-ID: | 4ec4a071-8f0a-5536-58d5-cb815ef0905f@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 4/26/23 6:06 AM, vignesh C wrote:
> On Tue, 25 Apr 2023 at 12:51, Drouvot, Bertrand
> <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> Thanks for the updated patch.
> Few comments:
Thanks for looking at it!
> 1) subscriber_stdout and subscriber_stderr are not required for this
> test case, we could remove it, I was able to remove those variables
> and run the test successfully:
> +$node_subscriber->start;
> +
> +my %psql_subscriber = (
> + 'subscriber_stdin' => '',
> + 'subscriber_stdout' => '',
> + 'subscriber_stderr' => '');
> +$psql_subscriber{run} = IPC::Run::start(
> + [ 'psql', '-XA', '-f', '-', '-d',
> $node_subscriber->connstr('postgres') ],
> + '<',
> + \$psql_subscriber{subscriber_stdin},
> + '>',
> + \$psql_subscriber{subscriber_stdout},
> + '2>',
> + \$psql_subscriber{subscriber_stderr},
> + $psql_timeout);
>
> I ran it like:
> my %psql_subscriber = (
> 'subscriber_stdin' => '');
> $psql_subscriber{run} = IPC::Run::start(
> [ 'psql', '-XA', '-f', '-', '-d', $node_subscriber->connstr('postgres') ],
> '<',
> \$psql_subscriber{subscriber_stdin},
> $psql_timeout);
>
Not using the 3 std* is also the case for example in 021_row_visibility.pl and 032_relfilenode_reuse.pl
where the "stderr" is set but does not seem to be used.
I don't think that's a problem to keep them all and I think it's better to have
them re-directed to dedicated places.
> 2) Also we have changed the default timeout here, why is this change required:
> my $node_cascading_standby =
> PostgreSQL::Test::Cluster->new('cascading_standby');
> +my $node_subscriber = PostgreSQL::Test::Cluster->new('subscriber');
> my $default_timeout = $PostgreSQL::Test::Utils::timeout_default;
> +my $psql_timeout = IPC::Run::timer(2 * $default_timeout);
I think I used 021_row_visibility.pl as an example. But agree there is
others .pl that are using the timeout_default as the psql_timeout and that
the default is enough in our case. So, using the default in V5 attached.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v5-0001-Add-subscribtion-to-the-standby-test-in-035_stand.patch | text/plain | 6.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Richard Guo | 2023-04-26 08:33:39 | Re: [pg_rewind] use the passing callback instead of global function |
Previous Message | Peter Eisentraut | 2023-04-26 07:38:40 | Re: run pgindent on a regular basis / scripted manner |