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

Thanks for the review.

On Tue, Nov 24, 2015 at 6:15 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
wrote:

> I just noticed that RecoveryTest.pm is lacking "use strict; use
> warnings;". With those added, there's a number of problems reported:
>
Most of them are easily fixable by adding the correct "my" lines; but at
> least @array and $current_dir require more code to be written.
>

Oops.

> TBH all that business with arrays that are kept in sync looks too
> contrived to me. Could we have a Perl object representing each node
> instead?
>

Not really to be honest.

> That would require a "PostgresNode" package (or similar). The
> RecoveryTest.pm would have a single %nodes hash. Also, you don't need
> @active_nodes, just a flag in PostgresNode, and have the stop routine do
> nothing if node is not marked active. Also: if you pass the "root node"
> when creating a node that will become a standby, you don't need to pass
> it when calling, say, enable_streaming; the root node becomes an
> instance variable. (Hmm, actually, if we do that, I wonder what if in
> the future we want to test node promotion and a standby is repointed to
> a new master. Maybe we don't want to have this knowledge in the Perl
> code at all.)
>

I think I'll get the idea. In short all the parametrization will just
happen at object level, as well as basic actions on the nodes like start,
stop, restart etc.

> In get_free_port, isn't it easier to use pg_isready rather than psql?
>

Will switch.

> I've been messing with 003 because I think it's a bit too repetitive.
> Will finish it after you post a fixed version of RecoveryTest.pm.
>

Sure, thanks.

I'll rework this patch and will update a new version soon.

Thanks again for the review.
--
Michael