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

Shutdown health check registry #1084

Merged
merged 3 commits into from
Feb 24, 2017
Merged

Conversation

arteam
Copy link
Member

@arteam arteam commented Feb 24, 2017

A follow-up for #1077

  • Add a method for graceful shutdown of HealthCheckRegistry
    Add an ability to gracefully shutdown the executor for async tasks in HealthCheckRegistry.
    Now it creates a ScheduledExecutorService which should be correctly closed when HealthCheckRegistry is not used anymore.

  • Make the pool size more pessimistic by default
    Some CI environments return the amount of available processors as 128.
    It's rather expensive to create such big thread pools by default. Let's start
    with 2 threads and the user wants to increase the amount, she can do that.

  • Make the async health check threads as daemons
    So they don't prevent the JVM from shutdown if the user didn't shut down
    the executor gracefully.

Add an ability to gracefully shutdown the executor for async tasks in `HealthCheckRegistry`.
Now it creates a ScheduledExecutorService which should be correctly closed when `HealthCheckRegistry`
is not used anymore.
Some CI environments return the amount of available processors as 128.
It's rather expensive to create such big thread pools by default. Let's start
with 2 threads and the user wants to increase the amount,  she can do that.
So they don't prevent the JVM from shutdown if the user didn't shut down
the executor gracefully.
@jplock jplock added this to the 3.2.0 milestone Feb 24, 2017
@jplock
Copy link
Member

jplock commented Feb 24, 2017

LGTM

@jplock jplock merged commit 4df3b39 into 3.2-development Feb 24, 2017
@jplock jplock deleted the shutdown_health_check_registry branch February 24, 2017 12:42
@arteam
Copy link
Member Author

arteam commented Feb 24, 2017

Bummer, I screwed up and this change was not added to 3.2.0. I will mark it for 3.2.1.

@arteam arteam modified the milestones: 3.2.1, 3.2.0 Feb 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants