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

During startup scheduled tasks are driven earlier than before causing problems with Spring Batch [SPR-12641] #17242

Closed
spring-issuemaster opened this issue Jan 19, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jan 19, 2015

Andy Wilkinson opened SPR-12641 and commented

Spring Batch performs it job registration in response to a ContextRefreshedEvent. In Spring 4.0, the scheduler starts calling scheduled tasks in response to a ContextRefreshedEvent. In Spring 4.1 the latter has changed due #16655 which made ScheduledAnnotationBeanPostProcessor a SmartInitializingSingleton. In short this means that the scheduler now starts calling scheduled tasks before the ContextRefreshedEvent is sent. This means that scheduled tasks that are driven during startup can no longer depend on a Batch job having been registered.


Affects: 4.1.4

Reference URL: spring-projects/spring-boot#2310

Issue Links:

  • #16655 JmsListener/ScheduledAnnotationBeanPostProcessor should use SmartInitializingSingleton instead of ContextRefreshedEvent

Referenced from: commits 0479ca6, 14a3bf3

0 votes, 6 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2015

Juergen Hoeller commented

I'm afraid there isn't much we can do on the Spring Framework side here, since the use of SmartInitializingSingleton is definitely preferable over ContextRefreshedEvent.

Which tasks typically depend on the registration of Batch jobs? Is there anything Spring Batch can do to register its jobs earlier, possibly in a SmartInitializingSingleton as well? Or can the task implementations be revised to wait for the next iteration if the Batch jobs aren't available yet?

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2015

Andy Wilkinson commented

The main problem here is that this change has caused a regression in an application that worked fine on Spring Boot 1.1.x which uses Spring 4.0.x. I'd like us to have a better answer than asking the user the change their code. Perhaps the Batch team can detect the presence of Spring 4.1 and do things earlier?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 21, 2015

Michael Minella commented

Juergen Hoeller, I had a chat with Andy Wilkinson and Dave Syer about this. In reading the documentation for SmartInitializingSingleton, it states:

NOTE: If you intend to start/manage asynchronous tasks, preferably implement Lifecycle instead which offers a richer model for runtime management and allows for phased startup/shutdown.

Isn't that exactly what is being done here? If so, I'd think that ScheduledAnnotationBeanPostProcessor shouldn't implement SmartInitializingSingleton.

Looking at the current batch code, the class in question is the AutomaticJobRegistrar. It currently implements both ApplicationListener and Lifecycle. The ApplicationListener#onApplicationEvent just delegates to the Lifecycle methods on the appropriate events (ContextRefreshedEvent and ContextClosedEvent). Without creating a component that implements SmartInitializingSingleton, we aren't sure how to beat your initialization if ScheduledAnnotationBeanPostProcessor remains a SmartInitializingSingleton.

At a minimum, if ScheduledAnnotationBeanPostProcessor remains a SmartInitializingSingleton, I'd think the javadoc in SmartInitializingSingleton should be updated to remove that note and we could use some insight into how to get ahead of the ScheduledAnnotationBeanPostProcessor. However, we'd prefer that the change to ScheduledAnnotationBeanPostProcessor that implemented SmartInitializingSingleton be reverted.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 22, 2015

Juergen Hoeller commented

Resolved though making ScheduledAnnotationBeanPostProcessor both a SmartInitializingSingleton and a ContextRefreshedEvent listener: When running in an ApplicationContext, it'll register its tasks in the ContextRefreshedEvent phase; when running in a plain BeanFactory, it'll happen at afterSingletonsInstantiated time. This should be a fine compromise across all intended scenarios now, even if the configuration state is now slightly more complex than I'd like it to be.

Please give this a try with Batch and Boot against the next 4.1.5 snapshot...

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 26, 2015

Andy Wilkinson commented

Thanks, Juergen. I can confirm that the problem reported in Spring Boot 2310 is fixed when run with 4.1.5.BUILD-20150123.212042-11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.