Introduce order for @EnableRetry #333
Conversation
|
Thank you for contribution; looking forward for more! |
| * <p> | ||
| * The default is {@link Ordered#LOWEST_PRECEDENCE} in order to run after all other | ||
| * post-processors, so that it can add an advisor to existing proxies rather than | ||
| * double-proxy. |
There was a problem hiding this comment.
Looks like there is an error in this description. The reasoning is misleading. The RetryConfiguration advice is not applied by any post-processors AFAIK. It is just registered as a bean to be applied later by bean-factory when it creates a proxy for a bean. And it's always applied just once, so there is no danger of creating double-proxy.
@artembilan Any chance this description could be corrected? To be honest I don't know the real reason of this property to be Ordered.LOWEST_PRECEDENCE except the backward compatibility. This default is actually danger as it interferes with @Transactional annotation that is also Ordered.LOWEST_PRECEDENCE, so the final order is undefined. But 99% of the time you want the order to be @Retry -> @Transactional and not vice versa because many exceptions will be thrown only at the commit phase and could be retried only by starting a new transaction.
There was a problem hiding this comment.
Sure! We can fix that description and also make a default order as a Ordered.LOWEST_PRECEDENCE - 1 to make it working with @Transactional as you have just explained which is really makes sense as a default behavior.
I think that description was taken from the @EnableAsync which looks like does its logic slightly different than an AspectJAwareAdvisorAutoProxyCreator initiated by the @EnableAspectJAutoProxy from this @EnableRetry.
Feel free to contribute the fix!
Thank you!
|
@artembilan Could you backport this and #335 to 1.3.x? many projects doesn't upgrade JDK to 17. |
|
Here it is: #372. I'll see what and how we can back-port it. Thanks |
Fix GH-22