-
Notifications
You must be signed in to change notification settings - Fork 38.4k
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
Phased Lifecycle Stop [SPR-5507] #10179
Comments
Ben Rowlands commented Shows a custom implementation of PhasedLifecycle. Running the example should log: [main] INFO example.PhasedLifecycleProcessor - Requesting start of bean b1 with order 1 |
Thomas Risberg commented Changes to the life-cycle start/stop behavior should also make the GenericApplicationContext behavior more consistent with the AbstractRefreshableConfigApplicationContext type contexts where isRunning() throws an IllegalStateException if called after the context has been closed. Currently the GenericApplicationContext returns true here since getBeanFactory() still returns the context. |
Mark Fisher commented We might want to consider this improvement for stop behavior along with potential improvements for start behavior. Specifically there are autoStartup cases where initialization is too early. Two relevant issues are #9393 and #10973. While the ApplicationListener alternative (reacting to a ContextRefreshedEvent) is better from a timing perspective, it might not be the most elegant solution to the problem. Perhaps we can address that issue while also working out the "phased" stops with something like "SmartLifecycle". |
Mark Fisher commented The stop(callback) method has been added to SmartLifecycle along with a getShutdownOrder() method that returns an int. The MessageListener containers now implement SmartFactoryBean and also provide a setShutdownOrder(int) method. The stopping of each phase is now controlled by DefaultLifecycleProcessor, the default implementation of a new strategy (LifecycleProcessor) in the ApplicationContext. Please try out your example with the upcoming 3.0 RC2 release, and let us know if this fully satisfies your requirements. Thanks, |
Matt Goldspink commented Hi Mark, I'll try this out in RC2. Is it possible to add some javadocs to the getShutdownOrder() and stop() methods to better clarify the contract? Copying from our own internal implementation we have something like:
For getShutdownOrder() it should also whether Integer.MIN_VALUE means that one stops first. In our implementation we actually use the Ordered interface from Spring and this then allowed to enforce ordered starting as well as stopping, have yuo considered whether adding ordered start callbacks are worth having too? We find it useful to ensure all our server transports (like TCP, JMS) are started after a users service is started. I know that Spring handles this via bean dependencies and it could also be enforced by making users use the depends-on bean attribute, but we found using getOrder() for start and stop more consistent. Matt |
Mark Fisher commented Thanks for the quick response. I just added javadoc to SmartLifecycle. The problem that I see with a single getOrder() is that start/stop order would often be reversed. In that sense, the "depends-on" setting is arguably more intuitive. Also, components might be implementing the Ordered interface for other reasons that have no relation to the Lifecycle. As you can see, we currently only support a single boolean for "auto" startup (triggered by the ApplicationContext refresh) rather than providing a getStartupOrder(). One idea is that any component that needs to start earlier can do so manually within the InitializingBean's afterPropertiesSet() callback while others (e.g. transport processes) would wait until the full context has been initialized. In the current implementation, the "depends-on" values are still being honored and actually take precedence over even the new shutdownOrder. |
Matt Goldspink commented
Absolutely and in fact that's the behaviour of our implementation. Ordered.MIN_PRIORITY is last to start and first to stop.
Agreed. I think we went with Ordered because it just seemed like the right interface to denote an order, but you're right a class might need Ordered to indicate the order for something else.
We did consider this but we wanted to make a clear distinction to the application creation stage (DI and bean creation) and then the more stable application lifecycle at which point everything is created and in a initialised ready state. To give an example if a user has a MessageListener on top of a DMLC and the service contains a cache of database information, then we would recommend that the user implements Lifecycle on their cache so that they don't start reading from the database until we're sure the container is stable. It also gives the added benefit that we can add JMX commands to lazily start a container after all the beans are created using the ApplicationContext start() and stop() events. I think the main use case we have for ordered start() is that we have some beans which are BeanPostProcessors and register certain types of bean with another component, for example we have a custom CacheManager (implementing BeanPostProcessor) which detects all beans of type Cache. The Cache interface has its own lifecycle methods: init() to prepare any settings, refresh() to populate it. We want init() called directly after the bean has been created (hence BeanPostProcessor.postProcessAfterInitialization() to call init()). Then we want refresh() called once start() is called on the Spring AppContext, so we make our CacheManager implement Lifecycle and give it an order so that it happens before any services try to use their Cache's to make them all fully populated. I admit this quite a bespoke issue which we hit while striving for back compatibility and trying to make it easier for our users to configure their Cache beans in Spring, but it would really help us if an ordered start callback was also available. This particular case it very difficult for us to use depends-on because we then need to make sure our CacheManager becomes a dependency of everything which has a Cache injected and would require us to scan pretty much every bean setter on all beans to figure that out which we really want to avoid. If I come up with other use cases for ordered start() I'll add them, but for now thats the main that comes to mind. |
Mark Fisher commented Reopening in order to replace 'shutdownOrder' with a single phase indicator value within SmartLifecycle. This value will be used for determining startup order (ascending) and then, in reverse direction (descending), shutdown order. On startup, we'll need to avoid the latency of using a latch for each phase as we do on shutdown, but for any blocking start() calls, the same basic effect would be achieved. |
Mark Fisher commented The default phase for any "normal" Lifecycle bean (one that does not implement SmartLifecycle) will be 0. Since the range of phases will run from Integer.MIN_VALUE to Integer.MAX_VALUE on start (and reverse on stop), this will provide the opportunity for SmartLifecycle beans to start either before (with a negative phase) or after (with a positive phase) those default Lifecycle beans. |
Mark Fisher commented The startup order is now driven by the phase value as well. See the latest changelog entry (by clicking on the "Source" tab above) for details, and please try out this code against the actual use-case. Looking forward to feedback! |
Matt Goldspink commented Hi Mark, This looks great and should definitely enable to use get rid our implementation. I will have to get a collegue to verify this as I'm on vacation the rest of the week, but I'll get him to update the thread once he's tested. Thanks again for this! |
Dave Syer commented Maybe you want to push this out to another issue, but it seems to me that the beans: namespace should also support a custom lifecycle: just like init-method= and destroy-method=, we should be providing start=method=, stop-method= and an adapter. The lifecycle processor could handle that. |
Mark Fisher commented Dave, Can you raise a new issue for start-method/stop-method? I think that would likely need to be a 3.0.1 or later addition. It also begs the question: do we need to provide annotations as well? |
Liu, Yinwei David commented Hi Mark, Matt, I've tested the SmartLifecycle, it works as what we wanted. Now, we can start/stop SmartLifecycle bean in the defined order. thanks. |
Mark Fisher commented Resolving for RC3. Anyone who has a chance to test this further with specific use-cases, please provide feedback here! Thanks. |
Ben Rowlands opened SPR-5507 and commented
Spring provides a Lifecycle interface (Start & Stop) in addition to a mechanism to broadcast these signals to beans in the application context.
#8436 suggested an improvement to allow beans to define the order that these signals are delivered. The fix in 2.5.2 (following dependency order) is arguably a neater solution. One caveat with deriving the order from Spring (rather than allowing the components to declare their own order) is when Spring can't "see" the dependencies. For example, if some BPP "magic" detects components and installs them into other components Spring can't detect this dependency. It does appear this can (and should) be solved with the BPP registering the dependencies it is creating explicitly (via registerDependency() API).
A more subtle problem with the Lifecycle is what 'stop' means (for most use-cases 'start' rarely causes problems). Indeed, its not clear from the documentation when the stop method should return. The choices are:
A) Stop the component and returns when stopped; effectively joining on "end-stop" event
B) Request the component to stop but returns immediately, possibly before it has stopped
The DMLC interprets "stop" as (B). This can cause problems since other dependant components can receive the stop signal before the DMLC is fully stopped (and may still be processing messages). If a DMLC is using a message handler that itself makes use of a database pool then the following can occur:
The problem defining "stop" as (A) is that large applications could take a long time shutdown. This can happen since components like the DMLC poll for work with a small timeout (in the order of 1 to 10 seconds). When the poll returns (and after any message is processed) the 'running' flag is checked to see if the thread should stop or carry on around the loop. Since the request for work can't be interrupted it can take up to the timeout interval for the thread to notice it should stop (assuming no work is available). To reduce load on our messaging queue manager we may run with a poll interval of say 10s. So each DMLC stop could take 10s and an application with 6 DMLCs could take 1 minute to shutdown.
To solve this phased shutdown could be introduced. For example, suppose we defined stop as:
/**
*/
public void stop(Runnable callback)
To shutdown the system all components with the same "phase" (defined by Ordered interface or otherwise) get the stop signal and we join on their completion. Once each phase is successfully stopped we continue to stop the next phase and so on. This approach allows us to concurrently shutdown phases whilst guaranteeing that components in earlier phases are already stopped. #9092 added such an API to the DMLC to allow for this.
It is possible to do this with our own PhasedLifecycle API and custom PhaseLifecycleProcessor (see attached files) however having this in Spring would help drive adoption of the Spring Lifecycle API and allow other components to participate in this model out the box (currently we have to wrap components manually).
Affects: 2.5.6
Attachments:
Issue Links:
Referenced from: commits 5e1c00c, 021663b, 5b088f5, b444220, 6ee17d2, 81efd48, 535ec5c, d5fd22c, a7c1f6b, a15a960
3 votes, 5 watchers
The text was updated successfully, but these errors were encountered: