reta commented on code in PR #3143:
URL: https://github.com/apache/cxf/pull/3143#discussion_r3314580092
##########
rt/features/clustering/src/main/java/org/apache/cxf/clustering/FailoverTargetSelector.java:
##########
@@ -113,6 +113,13 @@ &&
getClientBootstrapAddress().equals(message.get(Message.ENDPOINT_ADDRESS))) {
params,
context);
inProgress.putIfAbsent(key, invocation);
+
+ // CXF-9213: reset the per-exchange retry counter at the start of
+ // each new invocation so a singleton RetryStrategy starts from
zero.
+ if (getStrategy() instanceof RetryStrategy) {
Review Comment:
@ffang thanks for looking into this, I think it would be better to not
tangle RetryStrategy with exchanges and thread locals, what I have in mind (but
need some time to flash out) is something along this lines:
- introduce new interface `PerInvocationFailoverStrategy` - that would
indicate that the strategy is stateful
```java
interface PerInvocationFailoverStrategy {
FailoverStrategy newStrategy(InvocationContext context, Exchange
exchange);
}
```
- store it in map the like `inProgress` does:
```
private ConcurrentHashMap<String, FailoverStrategy> inProgressStrategies
= new ConcurrentHashMap<>();
```
- introduce `getStrategy(String key)` method to actually retrieve
`FailoverStrategy` per invocation, that will fallback to `getStrategy()` if is
not per invocation
- the `RetryStrategy` would implement `PerInvocationFailoverStrategy` and
return new fresh instance for each invocation
I think it should work, may be I am missing some details, but I believe it
will more maintainable solution, wdyt? Thanks again!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
