feat(java-client): Introduce exponentialBackoff retry mechanism #1822
Conversation
Samunroyu
changed the title
Samunroyu
changed the title
|
Some suggestions to improve the commit message: |
java-client/src/main/java/org/apache/pegasus/client/retry/DefaultRetryPolicy.java
Show resolved
Hide resolved
java-client/src/main/java/org/apache/pegasus/client/retry/DefaultRetryPolicy.java
Outdated
Show resolved
Hide resolved
| private final int retryMaxTimes; | ||
|
|
||
| public ExponentialBackoffRetryPolicy(ClientOptions opts) { | ||
| this.retryBaseIntervalMs = opts.getRetryBaseInterval().toMillis(); |
There was a problem hiding this comment.
Better to add some comments and also some checks when setting retry base interval, as here we will convert the timeout to milliseconds, if user specify an interval less than 1 millisecond, we will not perform as expected...
There was a problem hiding this comment.
Better to add some comments and also some checks when setting retry base interval, as here we will convert the timeout to milliseconds, if user specify an interval less than 1 millisecond, we will not perform as expected...
sure. if user specify an interval less than 1 millisecond, it will set to defaultintervalms.
Samunroyu
force-pushed
the
dev/yjw/retry
branch
from
8d16a58 to
cb92794
Compare
Samunroyu
changed the title
|
I tried rerun the failed job but it still failed... |
the failed job is failed at start pegasus cluster. not java client unit test. Rerun one more time the job successful. |
acelyc111
changed the title
acelyc111
changed the title
| long retryDelayMs = | ||
| Math.min( | ||
| timeoutMs > 3 ? timeoutMs / 3 : 1, TimeUnit.NANOSECONDS.toMillis(deadlineNanos - now)); | ||
| return new RetryAction(RetryDecision.RETRY, Duration.ofMillis(retryDelayMs), ""); |
There was a problem hiding this comment.
Since here we also pass a Duration in, let's use nanos instead of millis?
There was a problem hiding this comment.
Since here we also pass a Duration in, let's use nanos instead of millis?
So we should allow the user to specify the time interval for nano units?
There was a problem hiding this comment.
If we change to use nanos here, then we do not need to change the code here if we allow users to specify timeouts with smaller unit than millis, like micros and nanos in the future.
Whether to allow users to specify timeout with unit smaller than millis can be discussed later.
There was a problem hiding this comment.
If we change to use nanos here, then we do not need to change the code here if we allow users to specify timeouts with smaller unit than millis, like micros and nanos in the future. Whether to allow users to specify timeout with unit smaller than millis can be discussed later.
I see. i already change the code.pls take a look one more time.
| if (now >= deadlineNanos) { | ||
| return new RetryAction(RetryDecision.FAIL, Duration.ZERO, "request deadline reached"); | ||
| } | ||
| long normalIntervalMs = |
java-client/src/main/java/org/apache/pegasus/client/retry/ExponentialBackoffRetryPolicy.java
Outdated
Show resolved
Hide resolved
Introduces a retry mechanism into the Java client to enhance user
convenience and robustness.
It can be enabled by calling
retryPolicy(),retryBaseInterval()and
retryMaxTimes()in when construct ClientOptions objects.