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

Change phases of WebServer start-stop and graceful shutdown lifecycles to allow other smart lifecycles to run before graceful shutdown #31714

Closed
jGleitz opened this issue Jul 13, 2022 · 2 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@jGleitz
Copy link

jGleitz commented Jul 13, 2022

Use case: We would like to add some logic that runs on shutdown, but before the web server is shut down. To do so, we want to implement a SmartLifecycle with a phase that’s higher than WebServerGracefulShutdownLifecycle’s.

Our concrete use case is that we want to have the application wait some time before actually shutting down, as in #20995. However, the proposed solution in #20995 relies on hanging the thread that sends out the context closed event. We think that using the SmartLifecycle interface is more appropriate. However, since WebServerGracefulShutdownLifecycle’s phase is hard-coded to Integer.MAX_VALUE, we can’t do that.

Proposal: Add a property server.shutdown-phase to configure the SmartLifecycle phase of all WebServerGracefulShutdownLifecycles. Default the value to Integer.MAX_VALUE.

Alternative: Set the hard-coded phase of WebServerGracefulShutdownLifecycle to something lower than Integer.MAX_VALUE to allow using a higher phase. This is slightly less work, but is not strictly backwards-compatible, as it may influence the order in relation to other SmartLifecycles.

We would be happy to provide a pull request if one of the proposals is accepted.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 13, 2022
@jGleitz jGleitz changed the title Allow Configuring the Phase of WebServerGracefulShutdown Allow Configuring the Phase of WebServerGracefulShutdownLifecycle Jul 13, 2022
@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jul 18, 2022
@wilkinsona
Copy link
Member

We discussed this today and we'd prefer not to make the phase externally configurable as the phases of WebServerGracefulShutdownLifecycle and WebServerStartStopLifecycle need to be configured correctly in relation to each other. Instead, we'd prefer to change the defaults. Rather than using MAX_VALUE and MAX_VALUE - 1, we think it would be better to use something like MAX_VALUE - 5 and MAX_VALUE - 10.

@wilkinsona wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Aug 4, 2022
@wilkinsona wilkinsona added this to the 3.0.x milestone Aug 4, 2022
@wilkinsona wilkinsona changed the title Allow Configuring the Phase of WebServerGracefulShutdownLifecycle Change phases of WebServer start-stop and graceful shutdown lifecycles to allow other smart lifecycles to run before graceful shutdown Aug 4, 2022
@jGleitz
Copy link
Author

jGleitz commented Aug 9, 2022

Hi, and thank you for considering this.

Some feedback: if this is implemented by changing the hard-coded values, I’d suggest giving more headroom than just 5. In my experience, a lot of Lifecycles are configured to run one after the other, which can quickly fill up just 5 slots. I’d suggest something like MAX-VALUE - 32, or even less.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants