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

Michael Paquier wrote:
> On Thu, Nov 19, 2015 at 12:21 AM, Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)com> wrote:

> > Hi, I just started looking this over a bit. The first thing I noticed
> > is that it adds a dependency on Archive::Tar which isn't already used
> > anywhere else. Did anybody check whether this exists back in 5.8
> > installations?
>
> Actually I didn't and that's a good point, we have decided to support
> TAP down to 5.8.9. The only reason why I introduced this dependency is
> that there is no easy native way to copy an entire folder in perl, and
> that's for handling base backups. There are things like File::NCopy of
> File::Copy::Recursive however it does not seem like a good idea to
> depend on other modules that IPC::Run. Would it be better to have an
> in-core module dedicated to that similar to SimpleTee.pm? Or are you
> guys fine to accept a dependency with another module?

It would be a lot better to not have to rely on another module existing
everywhere. I'd rather have another simple module, following
SimpleTee's example. Since this doesn't have to be terribly generic, it
should be reasonably short, I hope.

> > Why is "recovery" added to ALWAYS_SUBDIRS in src/test/Makefile instead
> > of to SUBDIRS? Seems a strange choice.
>
> Because I thought that it should not be part of the main regression
> suite, like ssl/. Feel free to correct me if my feeling is wrong.

As I understand, the problem with "ssl" is that it messes with
system-wide settings, which is not the case here. I'm inclined to move
it to SUBDIRS. As an example, "modules" is not part of the main
regression suite either.

> > In my days of Perl, it was starting to become frowned upon to call
> > subroutines without parenthesizing arguments. Is that no longer the
> > case? Because I notice there are many places in this patch and pre-
> > existing that call psql with an argument list without parens. And it's
> > a bit odd because I couldn't find any other subroutine that we're using
> > in that way.
>
> Hm, yeah. If we decide about a perl coding policy I would be happy to
> follow it. Personally I prefer usually using parenthesis however if we
> decide to make the calls consistent we had better address that as a
> separate patch.

Some votes against, some votes for. Ultimately, it seems that this
depends on the committer. I don't really care all that much about this
TBH.

> > In 005_replay_delay there's a 2s delay configured; then we test whether
> > something is replayed in 1s. I hate tests that run for a long time, but
> > is 2s good enough considering that some of our test animals in buildfarm
> > are really slow?
>
> A call to poll_query_until ensures that we wait for the standby to
> replay once the minimum replay threshold is reached. Even with a slow
> machine the first query would still see only 10 rows at the first try,
> and then wait for the standby to replay before checking if 20 rows are
> visible. Or I am not following your point.

Ah, I see. Maybe it's fine then, or else I'm not following your point
;-)

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services