[
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 reused (eg
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 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-]
> 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 reused (eg
> 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)
