Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase timeout when checking for server stop #384

Merged
merged 1 commit into from
Dec 25, 2021

Conversation

sergiodj
Copy link
Contributor

On Ubuntu Impish (development version), we're currently facing a
problem where thin's testsuite fails when it runs on an armhf system.
After some investigation, I noticed that the reason for this failure
was that the server was still running even after the loop on
spec/spec_helper.rb:stop_server.

Given that the loop is actively checking to see if the server has
stopped, I'd like to propose that the timeout be increased from 1
second to 10 seconds. Although I'm not a fan of the
"let's-just-increase-the-timeout" type of solutions, in this
particular case I think it's justified and makes sense (1 second may
indeed be too quick for the server to properly stop).

On Ubuntu Impish (development version), we're currently facing a
problem where thin's testsuite fails when it runs on an armhf system.
After some investigation, I noticed that the reason for this failure
was that the server was still running even after the loop on
spec/spec_helper.rb:stop_server.

Given that the loop is actively checking to see if the server has
stopped, I'd like to propose that the timeout be increased from 1
second to 10 seconds.  Although I'm not a fan of the
"let's-just-increase-the-timeout" type of solutions, in this
particular case I think it's justified and makes sense (1 second may
indeed be too quick for the server to properly stop).

Signed-off-by: Sergio Durigan Junior <sergio.durigan@canonical.com>
@ioquatix
Copy link
Collaborator

ioquatix commented Dec 24, 2021

It seems wrong to me to use a fixed loop like this.

I would suggest using an exponential backoff and then exiting with an error if it's still not dead after several iterations. Otherwise if we just exit and it's still running as you say, it causes the test suite to hang/fail.

@sergiodj
Copy link
Contributor Author

It seems wrong to me to use a fixed loop like this.

I agree that the code is not optimal as is.

I would suggest using an exponential backoff and then exiting with an error if it's still not dead after several iterations. Otherwise if we just exit and it's still running as you say, it causes the test suite to hang/fail.

My proposal is just to increase the timeout so that we can mitigate the failures we're seeing on armhf. I can't promise I will have the time to work on anything more involved than that, I'm afraid.

@ioquatix
Copy link
Collaborator

Thanks for your effort. We can merge this, however this software is in maintenance mode FYI.

@ioquatix ioquatix merged commit 10607a2 into macournoyer:master Dec 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants