Allow setting shutdown timeout for all instances #4822
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ramonsmits
reviewed
Feb 18, 2025
ramonsmits
approved these changes
Feb 18, 2025
ramonsmits
reviewed
Feb 18, 2025
src/ServiceControlInstaller.Engine/Configuration/ServiceControl/ServiceControlAppConfig.cs
Outdated
Show resolved
Hide resolved
ramonsmits
reviewed
Feb 18, 2025
Member
|
Should this PR also update SCMU to set it to a higher value on upgrade? (since there we know its running as a win service) |
Member
Author
|
Sure thing, @andreasohlund. I'll continue working on it. |
andreasohlund
approved these changes
Mar 4, 2025
mauroservienti
commented
Mar 4, 2025
mauroservienti
changed the title
ramonsmits
approved these changes
Mar 5, 2025
mauroservienti
force-pushed
the
allow-setting-shutdown-timeout
branch
from
949a723 to
210121a
Compare
…ictive hosting platform
…ume it is Windows, and thus we can set the shutdown timeout to 2 minutes
…ng audit instances
Co-authored-by: Ramon Smits <ramon.smits@gmail.com>
…op to the audit instance
…OSPlatform` instead of suppressing CA1416
…pportedOSPlatform` instead of suppressing CA1416
mauroservienti
force-pushed
the
allow-setting-shutdown-timeout
branch
from
210121a to
da0f1fe
Compare
Member
Author
|
Rebased from |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR exposes a new setting (
ServiceControl*/ShutdownTimeout) to define the process shutdown timeout for all instances. The default value of 5 seconds has been selected based on the most restrictive hosting platform, containers on Linux. Linux containers allow for a maximum of ten seconds, and five were chosen to be as conservative as possible. That is less of an issue because when running ServiceControl using containers, we always run with an external RavenDB server, and the ServiceControl shutdown process is lightweight.On the other hand, when installing Windows as a service, either via SCMU or PowerShell, the installation process sets the shutdown timeout to 2 minutes. By default, Windows allows services to shut down for a maximum of 125 seconds. However, every service is given only 30 seconds unless it requests additional time. For that reason, this PR also includes a custom
WindowsServiceWithRequestTimeoutcomponent that requests additional time as defined by the newShutdownTimeoutsetting when the service control manager invokes the stop. Unfortunately, there is no way to request extra time when the operating system is shutting down; in that case, there is a high chance that the ServiceControl embedded database doesn't shut down cleanly.The new
WindowsServiceWithRequestTimeoutis registered when installing ServiceControl on Windows as a service and either run regularly or in maintenance mode.