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

On Fri, Apr 15, 2016 at 12:57 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Fri, Apr 15, 2016 at 12:39 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Fri, Apr 15, 2016 at 12:16 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>> On Thu, Apr 14, 2016 at 8:41 PM, Michael Paquier
>>> <michael.paquier@gmail.com> wrote:
>>>> On Thu, Apr 14, 2016 at 5:17 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>>>>> On Thu, Apr 14, 2016 at 1:22 PM, Michael Paquier
>>>>> <michael.paquier@gmail.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?

hamster can become easily quite busy to be honest.
-- 
Michael