[
https://issues.apache.org/jira/browse/CXF-9213?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Sammy Chan updated CXF-9213:
----------------------------
Description:
There's a trap in using RetryStrategy to retry failed calls.
RetryStrategy contains the counter field that makes it stateful, which is quite
unexpected for a strategy.
[https://github.com/apache/cxf/blob/cxf-4.2.1/rt/features/clustering/src/main/java/org/apache/cxf/clustering/RetryStrategy.java#L33]
When it is configured as a Singleton bean like its stateless cousins
SequentialStrategy and RandomStrategy as implied by the documentation:
[https://cxf.apache.org/docs/failoverfeature.html]
the consequence will be that retries of different (not necessarily concurrent
nor different thread) API calls will increment the SAME counter. As a result,
it will give up earlier than the maxNumberOfRetries set when all attempts fail
and when there are more than one thread doing retries. For example, let's say
maxNumberOfRetries = 5. One thread first calls API X, fails twice and succeed,
and then the same thread calls the same API X again, fails repeatedly for 3
times and then gives up unexpectedly, i.e the state lingers. The actual max no.
of retries become a random number between 1-5, which is very puzzling.
I think it's a design defect or at least a documentation one.
The documentation should make it clear it is stateful, not thread-safe and it
should be declared as whatever-scope so long as it won't be shared (eg
request/prototype)
Or better yet, the trap can be avoided at all from design. The RetryStrategy
should be stateless, current count should be passed from outside, like Apache
HttpClient (with execCount passed in):
[https://hc.apache.org/httpcomponents-client-5.6.x/5.6/httpclient5/apidocs/org/apache/hc/client5/http/HttpRequestRetryStrategy.html#retryRequest-org.apache.hc.core5.http.HttpRequest-java.io.IOException-int-org.apache.hc.core5.http.protocol.HttpContext-]
was:
There's a trap in using RetryStrategy to retry failed calls.
RetryStrategy contains the counter field that makes it stateful, which is quite
unexpected for a strategy.
[https://github.com/apache/cxf/blob/cxf-4.2.1/rt/features/clustering/src/main/java/org/apache/cxf/clustering/RetryStrategy.java#L33]
When it is configured as a Singleton bean like its stateless cousins
SequentialStrategy and RandomStrategy as implied by the documentation:
[https://cxf.apache.org/docs/failoverfeature.html]
the consequence will be that retries of different (not necessarily concurrent
nor different thread) API calls will increment the SAME counter. As a result,
it will give up earlier than the maxNumberOfRetries set when all attempts fail
and when there are more than one thread doing retries. For example, let's say
maxNumberOfRetries = 5. One thread first calls API X, fails twice and succeed,
and then the same thread calls the same API X again, fails repeatedly for 3
times and then gives up unexpectedly, i.e the state lingers. The behaviour
appears random and very puzzling.
I think it's a design defect or at least a documentation one.
The documentation should make it clear it is stateful, not thread-safe and it
should be declared as whatever-scope so long as it won't be shared (eg
request/prototype)
Or better yet, the trap can be avoided at all from design. The RetryStrategy
should be stateless, current count should be passed from outside, like Apache
HttpClient (with execCount passed in):
[https://hc.apache.org/httpcomponents-client-5.6.x/5.6/httpclient5/apidocs/org/apache/hc/client5/http/HttpRequestRetryStrategy.html#retryRequest-org.apache.hc.core5.http.HttpRequest-java.io.IOException-int-org.apache.hc.core5.http.protocol.HttpContext-]
> RetryStrategy is a stateful class whose object shouldn't be shared
> ------------------------------------------------------------------
>
> Key: CXF-9213
> URL: https://issues.apache.org/jira/browse/CXF-9213
> Project: CXF
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 4.2.1
> Reporter: Sammy Chan
> Assignee: Freeman Yue Fang
> Priority: Minor
>
> There's a trap in using RetryStrategy to retry failed calls.
> RetryStrategy contains the counter field that makes it stateful, which is
> quite unexpected for a strategy.
> [https://github.com/apache/cxf/blob/cxf-4.2.1/rt/features/clustering/src/main/java/org/apache/cxf/clustering/RetryStrategy.java#L33]
> When it is configured as a Singleton bean like its stateless cousins
> SequentialStrategy and RandomStrategy as implied by the documentation:
> [https://cxf.apache.org/docs/failoverfeature.html]
> the consequence will be that retries of different (not necessarily concurrent
> nor different thread) API calls will increment the SAME counter. As a result,
> it will give up earlier than the maxNumberOfRetries set when all attempts
> fail and when there are more than one thread doing retries. For example,
> let's say maxNumberOfRetries = 5. One thread first calls API X, fails twice
> and succeed, and then the same thread calls the same API X again, fails
> repeatedly for 3 times and then gives up unexpectedly, i.e the state lingers.
> The actual max no. of retries become a random number between 1-5, which is
> very puzzling.
> I think it's a design defect or at least a documentation one.
> The documentation should make it clear it is stateful, not thread-safe and it
> should be declared as whatever-scope so long as it won't be shared (eg
> request/prototype)
> Or better yet, the trap can be avoided at all from design. The RetryStrategy
> should be stateless, current count should be passed from outside, like Apache
> HttpClient (with execCount passed in):
> [https://hc.apache.org/httpcomponents-client-5.6.x/5.6/httpclient5/apidocs/org/apache/hc/client5/http/HttpRequestRetryStrategy.html#retryRequest-org.apache.hc.core5.http.HttpRequest-java.io.IOException-int-org.apache.hc.core5.http.protocol.HttpContext-]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
