Re: Add two missing tests in 035_standby_logical_decoding.pl

Hi,

On 4/26/23 6:06 AM, vignesh C wrote:
> On Tue, 25 Apr 2023 at 12:51, Drouvot, Bertrand
> <bertranddrouvot.pg@gmail.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