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

Fixes for graceful reload #72

Merged
merged 3 commits into from
Jul 2, 2019
Merged

Fixes for graceful reload #72

merged 3 commits into from
Jul 2, 2019

Conversation

nikmolnar
Copy link
Contributor

This PR fixes two issues with the newly-introduced graceful reload functionality.

The first fix addresses #71 by making reload an optional feature, disabled by default. The functionality can be enabled by passing the --enable-reload flag at startup.

The second fix addresses a problem that would cause the primary process to exit when near-concurrent reload requests were received (the newly forked process would receive a HUP signal before the handler was registered, which resulted in the default handler being invoked and exiting the process; this in turn prompted the parent process to exit as well).

The fix for this adds a .5s wait between reloads, to give the new child process time to start and register its HUP handler.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 8.962% when pulling a1eb5a6 on reload-fixes into 3d4efd5 on master.

Copy link
Collaborator

@brendan-ward brendan-ward left a comment

Choose a reason for hiding this comment

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

One typo to fix and otherwise ready to merge.

Thanks @nikmolnar, this was the direction I was hoping we would go in for this feature.

README.md Outdated Show resolved Hide resolved
@nikmolnar nikmolnar merged commit c873e74 into master Jul 2, 2019
@nikmolnar nikmolnar deleted the reload-fixes branch July 2, 2019 15:58
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.

3 participants