PlatformTransactionManager bean is exposed unconditionally [BATCH-2788]

[spring-projects-issues]

Stéphane Nicoll opened BATCH-2788 and commented

While this can't be changed easily as it will break backward compatibility, Spring Batch should not expose a PlatformTransactionManager unconditionally (see SimpleBatchConfiguration).

Spring Boot 2.1 has a mode enabled by default that throws an exception on startup if a bean is overridden. If a user defines its own transaction manager with Spring Batch, they will have to disable the flag as the application will fail. This isn't really expected as Spring Boot is supposed to handle and detect such scenario.

Arguably, Spring Boot should provide whatever PlatformTransactionManager and Spring Batch should be configured to use this. We can't implement this mode at the moment as Spring Batch exposes a PlatformTransactionManager itself.

This problem has been raised in several places (i.e. BATCH-2294). IMO, the resolution is incorrect: this is not about customizing the transaction manager to use, this is about exposing a bean in the context that may not be Spring Batch's responsibility in the first place. Besides, it is very likely that the transaction manager instance the user provides via the BatchConfigurer is already a bean so exposing it again feels wrong in the first place.

---

Affects: 4.1.1

[spring-projects-issues]

Michael Minella commented

A few initial thoughts on this:

1. Obviously that design predates Spring Boot and the logical ways (I'm aware of) to make this bean conditional are Boot based and not Spring Framework based (@ConditionalOnMissingBean for example).
2. Spring Batch does require a PlatformTransactionManager so this was an easy way to enforce that requirement.
3. At our level, we need to be concerned about users that are not Spring Boot users.  The mantra from Spring Boot was that it gets out of your way easily.  This feels like an example where it's not doing that.

This feels like an easy thing for Boot to address without the need for Spring Batch to make the required breaking changes that would impact non Spring Boot users but I'm open to be convinced otherwise.

[spring-projects-issues]

Stéphane Nicoll commented

This may have been phrased poorly but my report is not really related to Spring Boot per se. Exposing a PlatformTransactionManager bean does not seem adequate at the level of Spring Batch IMO.

This feels like an easy thing for Boot to address

Considering that @EnableBatchProcessing exposes a PlatformTransactionManager bean in the context with no way as far as I can see to prevent that, I didn't find a way which is why I created the issue in the first place.

[spring-projects-issues]

Michael Minella commented

For Spring Batch, we require a compatible transaction manager so the idea that we'd allow someone to turn it off completely doesn't make sense IMHO.  We already allow for facilities to customize which one you use, but those don't seem to be Boot friendly due to the ordering of operations in Boot.  This leaves us with the following (that we've been able to come up with...we're open to other alternatives):

1. Have Boot users not use @EnableBatchProcessing - We had to do this with Spring Cloud Task with the Spring Boot 2.1 release.  Spring Cloud Task imported beans that overrode what Boot did and had these same exceptions.  We ended up removing our static configuration (imported via an annotation) and used autoconfiguration for the same features.  This same approach could be used here, where Boot users would not use @EnableBatchProcessing and instead, Boot autoconfigures the components for the user via the batch starter.
2. Programmatically add the PlatformTransactionManager only if it's created by Spring Batch - Since Spring Batch 4 requires Spring Framework 5, we could tweak that code to not always create a PlatformTransactionManager, but instead, if one isn't configured to be  used, add it to the context programatically.  We'd have to explore the impacts on the APIs before we could commit to an approach like this.  This approach would still require BatchConfigurer customization in many cases including Spring Boot.

Any other mechanism Mahmoud and myself discussed means that we lose the ability to require a PlatformTransactionManager in a way that is both backwards compatible and maximizes the likelihood of compatibility.

[spring-projects-issues]

Stéphane Nicoll commented

Thanks for the feedback.

we require a compatible transaction manager so the idea that we'd allow someone to turn it off completely doesn't make sense IMHO.

This issue is about bean overriding and unconditional registration so hopefully nothing in my description lead to believe that I was asking to make this optional.

We already allow for facilities to customize which one you use, but those don't seem to be Boot friendly due to the ordering of operations in Boot.

That's an interesting way to present the current situation. This issue is not about customization but about unconditional bean exposure. Let me give you a couple examples to illustrate how this has nothing to do with Spring Boot:

• The user has a PlatformTransactionManager bean that they create themselves and provide it to Spring Batch as the transaction manager to use. Net outcome is that Spring Batch exposes that exact same instance again, either overriding the bean if they had the same name or with an additional name if the transaction manager of the user had a different name
• The user as an existing transactionManager bean name of another type for some reason and can't change the name so they create their PlatformTransactionManager with name platformTransactionManager and provide the instance to Spring Batch using the customizer so that they can retain their current arrangement. The user's bean is overridden

All in all, I think this statement is inaccurate. It's not about Spring Boot, it's not about ordering either.

Regarding the solutions:

• This is pushing the backward incompatible change in Spring Boot and so far Spring Batch has been opt-in and we'd like to keep it that way. We're ready to discuss that option if no other option is possible
• I think exposing a PlatformTransactionManager if you created it is definitely going in the right direction for everyone.

This approach would still require BatchConfigurer customization in many cases including Spring Boot.

Spring Boot is already doing this

 

[spring-projects-issues]

Michael Minella commented

Ok...it looks like we have at least an idea of how to proceed.  I'll keep you posted on what I see.

[fmbenhassine]

Related/Similar issue: #838.

[marcingrzejszczak]

With this issue in Sleuth (spring-cloud/spring-cloud-sleuth#1941) we've added support for instrumentation of PlatformTransactionManagers . I've ran the batch sample https://github.com/spring-cloud-samples/spring-cloud-sleuth-samples/tree/3.1.x/batch and the instrumented PTM wasn't called at all (even though it got instrumented). That means that currently Batch is ignoring any PTM wrapping and creates its beans outside of the standard Spring lifecycle (e.g. BeanPostProcessors can't warp its components).