Shall we revert test_resp_idle_timeout_shutdown_slow()?

[Fabiano Fidêncio]

This test was introduced in
https://github.com/SSSD/sssd/commit/ac9c3ad8228000140d80f91d4c5492d89d6e79f6
and its failing every now and then when running in our internal CI.

I'd like to have it reverted, at least for now, and re-added later
whenever we have a more stable CI or a more stable test.

Any objections?

Best Regards,
--
Fabiano Fidêncio

[Sumit Bose]

On Fri, May 04, 2018 at 09:57:51AM +0200, Fabiano Fidêncio wrote:
>  This test was introduced in
>  https://github.com/SSSD/sssd/commit/ac9c3ad8228000140d80f91d4c5492d89d6e79f6
>  and its failing every now and then when running in our internal CI.
>  
>  I'd like to have it reverted, at least for now, and re-added later
>  whenever we have a more stable CI or a more stable test.
>  
>  Any objections? 
In general I agree, but I wonder if test_resp_idle_timeout_shutdown_slow
would become more reliable if the timeout is just increased a bit. The
comment says:

# With the responder_idle_timeout set to 60 seconds, we need to wait at
# least 90, because the internal timer ticks every timeout/2 seconds, so
# so it would tick at 30, 60 and 90 seconds and the responder_idle_timeout
# uses a greater-than comparison, so the 60-seconds tick wouldn't yet
# trigger the process' shutdown.

So is the 60s tick is missed and SSSD will really run 90s using exactly
90 in p.wait(timeout=90) might be a bit on the edge. I wonder if you can
start some CI runs where e.g. p.wait(timeout=100) is used to see if
this will pass more reliable? Or is there a reason for the timeout being
exactly 90s?

bye,
Sumit

> 
> Best Regards,
> --
> Fabiano Fidêncio
> _______________________________________________
> sssd-devel mailing list -- sssd-devel(a)lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-leave(a)lists.fedorahosted.org

[Fabiano Fidêncio]

On Fri, May 4, 2018 at 10:20 AM, Sumit Bose <sbose(a)redhat.com&gt; wrote:
>  On Fri, May 04, 2018 at 09:57:51AM +0200, Fabiano Fidêncio wrote:
> > This test was introduced in
> > https://github.com/SSSD/sssd/commit/ac9c3ad8228000140d80f91d4c5492d89d6e79f6
> > and its failing every now and then when running in our internal CI.
> >
> > I'd like to have it reverted, at least for now, and re-added later
> > whenever we have a more stable CI or a more stable test.
> >
> > Any objections?
>
>  In general I agree, but I wonder if test_resp_idle_timeout_shutdown_slow
>  would become more reliable if the timeout is just increased a bit. The
>  comment says:
>
>  # With the responder_idle_timeout set to 60 seconds, we need to wait at
>  # least 90, because the internal timer ticks every timeout/2 seconds, so
>  # so it would tick at 30, 60 and 90 seconds and the responder_idle_timeout
>  # uses a greater-than comparison, so the 60-seconds tick wouldn't yet
>  # trigger the process' shutdown.
>
>  So is the 60s tick is missed and SSSD will really run 90s using exactly
>  90 in p.wait(timeout=90) might be a bit on the edge. I wonder if you can
>  start some CI runs where e.g. p.wait(timeout=100) is used to see if
>  this will pass more reliable? 
I can fire a bunch of jobs this afternoon and see the results on Monday.

>  Or is there a reason for the timeout being
>  exactly 90s? 
The reason is because the tests would be even slower, which is a
problem for some developers.



In any case, I'll increase the timeout and run 15~20 jobs over the weekend.
>
> bye,
> Sumit
>
>>
>> Best Regards,
>> --
>> Fabiano Fidêncio
>> _______________________________________________
>> sssd-devel mailing list -- sssd-devel(a)lists.fedorahosted.org
>> To unsubscribe send an email to sssd-devel-leave(a)lists.fedorahosted.org
> _______________________________________________
> sssd-devel mailing list -- sssd-devel(a)lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-leave(a)lists.fedorahosted.org

[Sumit Bose]

On Fri, May 04, 2018 at 11:17:26AM +0200, Fabiano Fidêncio wrote:
>  On Fri, May 4, 2018 at 10:20 AM, Sumit Bose <sbose(a)redhat.com&gt;
> wrote:
>  > On Fri, May 04, 2018 at 09:57:51AM +0200, Fabiano Fidêncio wrote:
>  >> This test was introduced in
>  >> https://github.com/SSSD/sssd/commit/ac9c3ad8228000140d80f91d4c5492d89d6e79f6
>  >> and its failing every now and then when running in our internal CI.
>  >>
>  >> I'd like to have it reverted, at least for now, and re-added later
>  >> whenever we have a more stable CI or a more stable test.
>  >>
>  >> Any objections?
>  >
>  > In general I agree, but I wonder if test_resp_idle_timeout_shutdown_slow
>  > would become more reliable if the timeout is just increased a bit. The
>  > comment says:
>  >
>  > # With the responder_idle_timeout set to 60 seconds, we need to wait at
>  > # least 90, because the internal timer ticks every timeout/2 seconds, so
>  > # so it would tick at 30, 60 and 90 seconds and the responder_idle_timeout
>  > # uses a greater-than comparison, so the 60-seconds tick wouldn't yet
>  > # trigger the process' shutdown.
>  >
>  > So is the 60s tick is missed and SSSD will really run 90s using exactly
>  > 90 in p.wait(timeout=90) might be a bit on the edge. I wonder if you can
>  > start some CI runs where e.g. p.wait(timeout=100) is used to see if
>  > this will pass more reliable?
>  
>  I can fire a bunch of jobs this afternoon and see the results on Monday.
>  
>  > Or is there a reason for the timeout being
>  > exactly 90s?
>  
>  The reason is because the tests would be even slower, which is a
>  problem for some developers. 
Yes, I agree, I do not like long running tests doing nothing either. But
if we want to test 'responder_idle_timeout=60' we have the wait the time
which is needed by SSSD to terminate properly. Btw, if the test would
pass there wouldn't be an additional delay because it will still run
about 90s. The extra 10s in the timeout would just cover some scheduling
weirdnesses on the test platform. In case of a failure (i.e. the process
is not stopped in the expected time) the test would of course run 10s
longer.

>  
>  
>  
>  In any case, I'll increase the timeout and run 15~20 jobs over the weekend. 
Thank you.

bye,
Sumit

> >
> > bye,
> > Sumit
> >
> >>
> >> Best Regards,
> >> --
> >> Fabiano Fidêncio
> >> _______________________________________________
> >> sssd-devel mailing list -- sssd-devel(a)lists.fedorahosted.org
> >> To unsubscribe send an email to sssd-devel-leave(a)lists.fedorahosted.org
> > _______________________________________________
> > sssd-devel mailing list -- sssd-devel(a)lists.fedorahosted.org
> > To unsubscribe send an email to sssd-devel-leave(a)lists.fedorahosted.org
> _______________________________________________
> sssd-devel mailing list -- sssd-devel(a)lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-leave(a)lists.fedorahosted.org

[Jakub Hrozek]

On Fri, May 04, 2018 at 11:42:48AM +0200, Sumit Bose wrote:
>  On Fri, May 04, 2018 at 11:17:26AM +0200, Fabiano Fidêncio wrote:
>  > On Fri, May 4, 2018 at 10:20 AM, Sumit Bose <sbose(a)redhat.com&gt; wrote:
>  > > On Fri, May 04, 2018 at 09:57:51AM +0200, Fabiano Fidêncio wrote:
>  > >> This test was introduced in
>  > >>
> https://github.com/SSSD/sssd/commit/ac9c3ad8228000140d80f91d4c5492d89d6e79f6
>  > >> and its failing every now and then when running in our internal CI.
>  > >>
>  > >> I'd like to have it reverted, at least for now, and re-added later
>  > >> whenever we have a more stable CI or a more stable test.
>  > >>
>  > >> Any objections?
>  > >
>  > > In general I agree, but I wonder if test_resp_idle_timeout_shutdown_slow
>  > > would become more reliable if the timeout is just increased a bit. The
>  > > comment says:
>  > >
>  > > # With the responder_idle_timeout set to 60 seconds, we need to wait at
>  > > # least 90, because the internal timer ticks every timeout/2 seconds, so
>  > > # so it would tick at 30, 60 and 90 seconds and the responder_idle_timeout
>  > > # uses a greater-than comparison, so the 60-seconds tick wouldn't yet
>  > > # trigger the process' shutdown.
>  > >
>  > > So is the 60s tick is missed and SSSD will really run 90s using exactly
>  > > 90 in p.wait(timeout=90) might be a bit on the edge. I wonder if you can
>  > > start some CI runs where e.g. p.wait(timeout=100) is used to see if
>  > > this will pass more reliable?
>  > 
>  > I can fire a bunch of jobs this afternoon and see the results on Monday.
>  > 
>  > > Or is there a reason for the timeout being
>  > > exactly 90s?
>  > 
>  > The reason is because the tests would be even slower, which is a
>  > problem for some developers.
>  
>  Yes, I agree, I do not like long running tests doing nothing either.  
btw we discussed this on the #sssd channel on IRC but no here on the
list, so I don't know if everyone had a chance to get involved. But if
slow tests are a problem, we can move them to some non-default tier and
only execute the 'fast' tests by default and only execute everything in
post-commit.

I was OK with the slow test mostly because I rarely ever run all tests
locally, mostly I only run whatever tests are relevant to the code I
changed.