Re: [REVIEW] In-core regression tests for replication, cascading, archiving, PITR, etc.

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Victor Wagner <vitus(at)wagner(dot)pp(dot)ru>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [REVIEW] In-core regression tests for replication, cascading, archiving, PITR, etc.
Date: 2016-02-17 12:52:06
Message-ID: CAB7nPqTmseK6KbuwiAE5JAW=uEWALzTekQkhaesoQxH8Bm_=mA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Feb 5, 2016 at 4:17 AM, Michael Paquier wrote:
> Thanks for your enthusiasm. Now, to do an auto-critic of my patch:
> + if ($params{allows_streaming})
> + {
> + print $conf "wal_level = hot_standby\n";
> + print $conf "max_wal_senders = 5\n";
> + print $conf "wal_keep_segments = 20\n";
> + print $conf "max_wal_size = 128MB\n";
> + print $conf "shared_buffers = 1MB\n";
> + print $conf "wal_log_hints = on\n";
> + print $conf "hot_standby = on\n";
> + }
> This could have more thoughts, particularly for wal_log_hints which is
> not used all the time, I think that we'd actually want to complete
> that with an optional hash of parameter/values that get appended at
> the end of the configuration file, then pass wal_log_hints in the
> tests where it is needed. The default set of parameter is maybe fine
> if done this way, still wal_keep_segments could be removed.

At the end I have refrained from doing that, and refactoring
setup_cluster(at)RewindTest(dot)pm to use the new option allows_streaming,
simplifying a bit the code. The introduction of allows_streaming could
be done in a separate patch, though it did not seem worth the
complication when hacking at that. The split is simple, though.

> +# Tets for timeline switch
> +# Encure that a standby is able to follow a newly-promoted standby
> Two typos in two lines.

Fixed.

I also found an issue with the use of application_name causing test
001 to fail, bug squashed on the way. The generation of paths for
archive_command and restore_command was incorrect on Windows. Those
need to use two backslashes (to detect correctly files) and need to be
double-quoted (to avoid errors with command copy should a space be
included in the path). I have added as well a new subcommand in
vcregress.pl called recoverycheck where one can run the recovery test
suite on Windows using MSVC.

Attached are rebased patches, split into 3 parts doing the following:
- 0001, fix default configuration of MSVC builds ignoring TAP tests
- 0002, add a promote routine in PostgresNode.pm. pg_rewind's tests
can make immediate use of that.
- 0003, the actual test suite.
This is registered in CF 2016-03 as well for further consideration.
--
Michael

Attachment Content-Type Size
0001-Fix-default-configuration-of-MSVC-builds-ignoring-TA.patch text/x-patch 3.0 KB
0002-Add-routine-for-node-promotion-in-PostgresNode.patch text/x-patch 1.7 KB
0003-Add-recovery-test-suite.patch text/x-patch 26.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2016-02-17 12:58:45 Re: pglogical - logical replication contrib module
Previous Message Pavel Stehule 2016-02-17 12:26:53 Re: commitfest application doesn't see new patch