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

On Fri, Apr 15, 2016 at 1:12 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Fri, Apr 15, 2016 at 12:57 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> 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?
>
> hamster can become easily quite busy to be honest.

Okay, since the test would fail very rarely (I hope),
probably we can live with 30 seconds timeout (it happens
only when the test fails).

I pushed your latest patch. Thanks!

Regards,

--
Fujii Masao