Re: [COMMITTERS] pgsql: Add regression tests for multiple synchronous standbys.

On Fri, Apr 15, 2016 at 12:39 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Fri, Apr 15, 2016 at 12:16 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> On Thu, Apr 14, 2016 at 8:41 PM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>>> On Thu, Apr 14, 2016 at 5:17 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>>>> On Thu, Apr 14, 2016 at 1:22 PM, Michael Paquier
>>>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>>>>> Yes. I'd prefer avoid a hardcoded sleep and have something that relies
>>>>> on lookups of pg_stat_replication though, because there is no way to
>>>>> be sure that a sleep will ever be stable on heavily loaded machines,
>>>>> like the machines I am specialized in maintaining :)
>>>>> It kills a bit the purpose on having checks on pg_stat_replication as
>>>>> the validation tests are based on that, still I think that we had
>>>>> better base those checks on a loop that has a timeout, something like
>>>>> that in a subroutine:
>>>>> What do you think?
>>>>
>>>> Look good to me. +1.
>>
>> +1 from me.
>>
>>> And so here we go...
>>
>> + ok($test_passed, $msg);
>>
>> ISTM that this change prevents the test from outputting the difference
>> of expected and actual results when the test fails. Which would make
>> the diagnosis of the test failure more difficult, I'm afraid.
>
> Well, then, it is just a matter of saving the result in a variable
> defined out of the loop, and use is() for the test. This way, after
> the timeout it is possible to check if the expected result and the
> fetched result match properly or not. In other words see attached.

The patch looks good to me.

+ my $timeout_max = 30;

One comment is; isn't 30 (seconds) too large value for the timeout?
What about just, say, 5?

Regards,

--
Fujii Masao