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

On Wed, Feb 17, 2016 at 9:52 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> 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@RewindTest.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.

Here is a rebased set after the conflicts created by e640093, with the
following changes:
- In 0002, added perldoc for new promote routine
- In 0003, added perldoc documentation for the new options introduced
in init and init_from_backup, and fixed some log entries not using the
node name to identify the node involved when enabling archive,
streaming or recovery.
- Craig has pinged me regarding tap_tests being incorrectly updated in
config_default.pl in 0003.
I just re-ran the tests on OSX and Windows (MSVC 2010 with Win7) to be
sure that nothing broke, and nothing has been reported as broken.
--
Michael