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

Add debug to worker timeout and startup. Closes #2528 #2559

Merged
merged 8 commits into from Mar 11, 2021

Conversation

ylecuyer
Copy link
Contributor

Description

Fixes #2528 adding the reason for worker timeout and the startup time when worker boots

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] or [ci skip] to the pull request title.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

@@ -46,6 +46,10 @@ def teardown

private

def silent_and_checked_system_command(*args)
assert(system(*args, out: File::NULL, err: File::NULL))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed err as bundler keeps on complaining my version is not the same as the one in the Gemfile.lock
I also added an assert to make sure the command exited with success

History.md Outdated Show resolved Hide resolved
Copy link
Member

@dentarg dentarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@dentarg dentarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like failing tests needs to be addressed

@ylecuyer
Copy link
Contributor Author

Looks like failing tests needs to be addressed

I have fixed the ruby < 2.4 versions. Sadly I can't test locally as I don't have the right openssl version to compile the old rubies. Let's see if the CI is happy this time ;)

@nateberkopec nateberkopec added feature waiting-for-review Waiting on review from anyone labels Feb 22, 2021
@MSP-Greg
Copy link
Member

MSP-Greg commented Mar 6, 2021

CI is fine, current CI has intermittent failures/errors for all platforms/versions. See:

https://github.com/MSP-Greg/puma/actions/runs/626438062, failures here passed, ubuntu-18.04 2.6 failed.

@cjlarose - given #2528, if you have time to review...

lib/puma/cluster.rb Outdated Show resolved Hide resolved
@dentarg
Copy link
Member

dentarg commented Mar 6, 2021

Sadly I can't test locally as I don't have the right openssl version to compile the old rubies

If you can use Docker, it is as simple as docker run --rm -it ruby:2.2 (it will start irb as default)

Co-authored-by: Patrik Ragnarsson <patrik@starkast.net>
@cjlarose
Copy link
Member

cjlarose commented Mar 8, 2021

@cjlarose - given #2528, if you have time to review...

Oh man I forgot I wrote that issue. I'll do some local testing on this, but on the surface, this looks great!

Copy link
Member

@cjlarose cjlarose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work. Thanks for doing this!

Copy link
Member

@dentarg dentarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the CI failures are not related

(18.04 2.7 flaky test_hot_restart_does_not_drop_connections and Win 2019 2.3 SignalException: SIGSEGV, flake too?)

@MSP-Greg MSP-Greg merged commit c68e20d into puma:master Mar 11, 2021
JuanitoFatas pushed a commit to JuanitoFatas/puma that referenced this pull request Sep 9, 2022
Logs reason for worker timeout and the startup time when worker boots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature waiting-for-review Waiting on review from anyone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve log messages to help debug worker timeouts
5 participants