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

Restarting a service when any hook changes is too extreme and leads to unnecessary restarts #5305

Closed
christophermaier opened this issue Jul 10, 2018 · 1 comment
Assignees
Labels
Focus:Supervisor ProcessManagement Related to how the Supervisor manages service processes Focus:Supervisor Related to the Habitat Supervisor (core/hab-sup) component Type:Breaking Change PRs that are classified as a change to existing behavior Type: Bug Issues that describe broken functionality

Comments

@christophermaier
Copy link
Contributor

christophermaier commented Jul 10, 2018

Currently, whenever any lifecycle hook file is updated in response to census changes, we trigger a restart of the service.

Based on how the hooks in question are actually run, this is actually far too aggressive, and could lead to services restarting more than is strictly necessary.

For example, the post-stop, suitability, file_updated, and health_check hooks are all run on-demand, and actually outside of the "main loop" of the service hook lifecycle. They cannot, by definition, impact how a service process runs. Yet, if any of them change, we restart the service.

Similarly, the init hook is only run when the service is first loaded, or when it has been updated with a new release from Builder. It does not run each time before the main run hook does. Therefore, any changes to this hook are actually completely irrelevant, because it won't be run again anyway.

The reload hook is a bit of an outlier (see #5306, #5307), and a change to the reconfigure hook seems like it should just trigger a reconfiguration, rather than a restart, since that's its sole purpose.

Changes to the run hook should definitely trigger a restart. Changes to post-run could also trigger a restart (but also see #2364).

@baumanj
Copy link
Contributor

baumanj commented Jul 11, 2018

second
"Second"

@christophermaier christophermaier added the Focus:Supervisor ProcessManagement Related to how the Supervisor manages service processes label Nov 30, 2018
@jsirex jsirex mentioned this issue Dec 6, 2018
@davidMcneil davidMcneil self-assigned this Jun 4, 2019
davidMcneil added a commit that referenced this issue Jun 18, 2019
Resolves #5305 #5306 #5307

Signed-off-by: David McNeil <mcneil.david2@gmail.com>
davidMcneil added a commit that referenced this issue Jun 20, 2019
Resolves #5305 #5306 #5307

Signed-off-by: David McNeil <mcneil.david2@gmail.com>

PR feedback

Signed-off-by: David McNeil <mcneil.david2@gmail.com>
davidMcneil added a commit that referenced this issue Jun 21, 2019
Resolves #5305 #5306 #5307

Signed-off-by: David McNeil <mcneil.david2@gmail.com>

PR feedback

Signed-off-by: David McNeil <mcneil.david2@gmail.com>
davidMcneil added a commit that referenced this issue Jun 26, 2019
Resolves #5305 #5306 #5307

Signed-off-by: David McNeil <mcneil.david2@gmail.com>

PR feedback

Signed-off-by: David McNeil <mcneil.david2@gmail.com>
davidMcneil added a commit that referenced this issue Jul 2, 2019
Resolves #5305 #5306 #5307

Signed-off-by: David McNeil <mcneil.david2@gmail.com>

PR feedback

Signed-off-by: David McNeil <mcneil.david2@gmail.com>
davidMcneil added a commit that referenced this issue Jul 7, 2019
Resolves #5305 #5306 #5307

Signed-off-by: David McNeil <mcneil.david2@gmail.com>

PR feedback

Signed-off-by: David McNeil <mcneil.david2@gmail.com>
davidMcneil added a commit that referenced this issue Jul 8, 2019
Resolves #5305 #5306 #5307

Signed-off-by: David McNeil <mcneil.david2@gmail.com>

PR feedback

Signed-off-by: David McNeil <mcneil.david2@gmail.com>
@christophermaier christophermaier added Type:Breaking Change PRs that are classified as a change to existing behavior Focus:Supervisor Related to the Habitat Supervisor (core/hab-sup) component Type: Bug Issues that describe broken functionality and removed X-change labels Jul 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Focus:Supervisor ProcessManagement Related to how the Supervisor manages service processes Focus:Supervisor Related to the Habitat Supervisor (core/hab-sup) component Type:Breaking Change PRs that are classified as a change to existing behavior Type: Bug Issues that describe broken functionality
Projects
None yet
Development

No branches or pull requests

4 participants