Skip to content
Closed
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
c1ef667
[WIP] feat: add hedging ability to RetryingRpcClient with first tests
schiemon May 27, 2025
27e22d7
refactor: improve variable naming and reduce method parameter count i…
schiemon Jun 2, 2025
057e441
[WIP] feat: add hedging ability to retrying (rpc) client
schiemon Jun 6, 2025
b74afca
[WIP] feat: make scheduler thread-safe and add test when hedging task…
schiemon Jun 7, 2025
fe9f063
[WIP] feat: add test verifying that RetryingClient stops hedging afte…
schiemon Jun 7, 2025
fbc4455
[WIP] feat: add test verifying that RetryingClient stops correctly af…
schiemon Jun 8, 2025
60ea9a8
[WIP] feat: change hedging API to accept long or Duration instead of …
schiemon Jun 8, 2025
835b268
[WIP] test: add `RetryScheduler` tests
schiemon Jun 10, 2025
5fe93c7
[WIP] fix, test: tackle concurrency issues in RetryingClient, simplif…
schiemon Jun 14, 2025
d7de984
[WIP] style: fix checkstyle errors
schiemon Jun 14, 2025
7862f28
[WIP] style: fix checkstyle errors
schiemon Jun 14, 2025
4d97a1b
[WIP] fix: fix exception message for RETRY_TASK_CANCELLED
schiemon Jun 14, 2025
4e2b540
[WIP] test: add firstServerWins to RetryingClientWithHedgingTest
schiemon Jun 14, 2025
f0c63b3
[WIP] fix: RetryingRpcClient scheduling exception handling
schiemon Jun 15, 2025
072e3e4
[WIP] style: remove comment in ClientUtil.newDerivedContext
schiemon Jun 15, 2025
785c384
[WIP] style: remove comment in RetryScheduler
schiemon Jun 15, 2025
17b9645
[WIP] test: add reminder in RetrySchedulerTest for testing reschedule…
schiemon Jun 15, 2025
a4f302c
[WIP] test: add TODO comments for tests in RetryingClient
schiemon Jun 16, 2025
6a3db67
[WIP] test: add thirdServerWinsEvenAfterPerAttemptTimeout to Retrying…
schiemon Jun 17, 2025
4db9525
[WIP] feat, test: make sure each attempt is started with a consistent…
schiemon Jun 17, 2025
5625e75
[WIP] refactor: some renaming for consistency
schiemon Jun 17, 2025
de47889
test: remove unnecessary change from doNotRetryWhenResponseIsCancelled
schiemon Jun 18, 2025
9914f78
test: add log checks to RetryingClientTest
schiemon Jun 18, 2025
3853d9f
refactor: rename variable in propagateResponseSideLog
schiemon Jun 18, 2025
6d6044a
docs: comment on why retry task scheduling should be correct
schiemon Jun 18, 2025
164aa2d
fix: leak failure in tests because of unfinished responses of aborted…
schiemon Jun 19, 2025
f130632
Merge branch 'main' into 5200-introduce-grpc-request-hedging
schiemon Jun 19, 2025
5efaf49
fix: RetryingRpcClientWithHedgingTest
schiemon Jun 19, 2025
42c870c
fix: give scheduler in RetrySchedulerTest more wiggleroom
schiemon Jun 19, 2025
9f849b0
fix: RetryingRpcClientWithHedgingTest.loosesAfterResponseTimeout
schiemon Jun 19, 2025
431b927
fix: reduce state space by early exists in RetryingClient
schiemon Jun 19, 2025
b610027
fix: remove limit for `RetryingClientWithHedgingTest.loosesAfterRespo…
schiemon Jun 19, 2025
5e6a867
fix: schedule winning handler on attempt event loop RetryingRpcClient
schiemon Jun 20, 2025
b063104
test: swap asserts to get more insight on failure
schiemon Jun 20, 2025
7927a92
fix: doNotRetryWhenSubscriberIsCancelled
schiemon Jun 20, 2025
c5d93e1
refactor: revert updates to copyright headers
schiemon Jun 25, 2025
7e64991
Merge branch 'main' into 5200-introduce-grpc-request-hedging
schiemon Jun 25, 2025
9c1ecca
refactor: revert updates to copyright headers and further cleanups
schiemon Jun 25, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* Copyright 2017 LINE Corporation
* Copyright 2025 LY Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* LY Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
Expand Down Expand Up @@ -187,7 +187,7 @@ static RandomBackoffBuilder builderForRandom() {
}

/**
* Returns the number of milliseconds to wait for before attempting a retry.
* Returns the number of milliseconds to wait for before attempting a retry. This method is idempotent.
*
* @param numAttemptsSoFar the number of attempts made by a client so far, including the first attempt and
* its following retries.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* Copyright 2020 LINE Corporation
* Copyright 2025 LY Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* LY Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
Expand Down Expand Up @@ -77,6 +77,7 @@ static <T extends Response> RetryConfigBuilder<T> builder0(

private final int maxTotalAttempts;
private final long responseTimeoutMillisForEachAttempt;
private final long hedgingDelayMillis;
private final int maxContentLength;

@Nullable
Expand All @@ -90,7 +91,16 @@ static <T extends Response> RetryConfigBuilder<T> builder0(

RetryConfig(RetryRule retryRule, int maxTotalAttempts, long responseTimeoutMillisForEachAttempt) {
this(requireNonNull(retryRule, "retryRule"), null,
maxTotalAttempts, responseTimeoutMillisForEachAttempt, 0);
maxTotalAttempts, responseTimeoutMillisForEachAttempt,
0, -1);
checkArguments(maxTotalAttempts, responseTimeoutMillisForEachAttempt);
}

RetryConfig(RetryRule retryRule, int maxTotalAttempts, long responseTimeoutMillisForEachAttempt,
long hedgingDelayMillis) {
this(requireNonNull(retryRule, "retryRule"), null,
maxTotalAttempts, responseTimeoutMillisForEachAttempt,
0, hedgingDelayMillis);
checkArguments(maxTotalAttempts, responseTimeoutMillisForEachAttempt);
}

Expand All @@ -100,21 +110,36 @@ static <T extends Response> RetryConfigBuilder<T> builder0(
int maxTotalAttempts,
long responseTimeoutMillisForEachAttempt) {
this(null, requireNonNull(retryRuleWithContent, "retryRuleWithContent"),
maxTotalAttempts, responseTimeoutMillisForEachAttempt, maxContentLength);
maxTotalAttempts, responseTimeoutMillisForEachAttempt,
maxContentLength, -1);
}

RetryConfig(
RetryRuleWithContent<T> retryRuleWithContent,
int maxContentLength,
int maxTotalAttempts,
long responseTimeoutMillisForEachAttempt,
long hedgingDelayMillis) {
this(null, requireNonNull(retryRuleWithContent, "retryRuleWithContent"),
maxTotalAttempts, responseTimeoutMillisForEachAttempt,
maxContentLength, hedgingDelayMillis);
}

private RetryConfig(
@Nullable RetryRule retryRule,
@Nullable RetryRuleWithContent<T> retryRuleWithContent,
int maxTotalAttempts,
long responseTimeoutMillisForEachAttempt,
int maxContentLength) {
int maxContentLength,
long hedgingDelayMillis
) {
checkArguments(maxTotalAttempts, responseTimeoutMillisForEachAttempt);
this.retryRule = retryRule;
this.retryRuleWithContent = retryRuleWithContent;
this.maxTotalAttempts = maxTotalAttempts;
this.responseTimeoutMillisForEachAttempt = responseTimeoutMillisForEachAttempt;
this.maxContentLength = maxContentLength;
this.hedgingDelayMillis = hedgingDelayMillis;
if (retryRuleWithContent == null) {
fromRetryRuleWithContent = null;
} else {
Expand Down Expand Up @@ -145,9 +170,15 @@ public RetryConfigBuilder<T> toBuilder() {
assert retryRule != null;
builder = builder0(retryRule);
}
return builder
builder
.maxTotalAttempts(maxTotalAttempts)
.responseTimeoutMillisForEachAttempt(responseTimeoutMillisForEachAttempt);

if (hedgingDelayMillis >= 0) {
builder.hedgingDelayMillis(hedgingDelayMillis);
}

return builder;
}

/**
Expand All @@ -166,6 +197,13 @@ public long responseTimeoutMillisForEachAttempt() {
return responseTimeoutMillisForEachAttempt;
}

/**
* todo(szymon): [doc].
*/
public long hedgingDelayMillis() {
return hedgingDelayMillis;
}

/**
* Returns the {@link RetryRule} which was specified with {@link RetryConfig#builder(RetryRule)}.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* Copyright 2020 LINE Corporation
* Copyright 2025 LY Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* LY Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
Expand Down Expand Up @@ -36,6 +36,7 @@
public final class RetryConfigBuilder<T extends Response> {
private int maxTotalAttempts = Flags.defaultMaxTotalAttempts();
private long responseTimeoutMillisForEachAttempt = Flags.defaultResponseTimeoutMillis();
private long hedgingDelayMillis = -1;
private int maxContentLength;

@Nullable
Expand Down Expand Up @@ -84,6 +85,33 @@
return this;
}

/**
* todo(szymon): [doc].
*/
public RetryConfigBuilder<T> hedgingDelay(Duration hedgingDelay) {
final long millis =
requireNonNull(hedgingDelay, "hedgingDelay")
.toMillis();

Check warning on line 94 in core/src/main/java/com/linecorp/armeria/client/retry/RetryConfigBuilder.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/com/linecorp/armeria/client/retry/RetryConfigBuilder.java#L92-L94

Added lines #L92 - L94 were not covered by tests
checkArgument(
millis >= 0,
"responseTimeoutForEachAttempt.toMillis(): %s (expected: >= 0)",
millis);
hedgingDelayMillis = millis;
return this;

Check warning on line 100 in core/src/main/java/com/linecorp/armeria/client/retry/RetryConfigBuilder.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/com/linecorp/armeria/client/retry/RetryConfigBuilder.java#L99-L100

Added lines #L99 - L100 were not covered by tests
}

/**
* todo(szymon): [doc].
*/
public RetryConfigBuilder<T> hedgingDelayMillis(long hedgingDelayMillis) {
checkArgument(
hedgingDelayMillis >= 0,
"hedgingDelayMillis: %s (expected: >= 0)",
hedgingDelayMillis);
this.hedgingDelayMillis = hedgingDelayMillis;
return this;
}

/**
* Sets the specified {@code responseTimeoutMillisForEachAttempt}.
*/
Expand Down Expand Up @@ -116,14 +144,22 @@
*/
public RetryConfig<T> build() {
if (retryRule != null) {
return new RetryConfig<>(retryRule, maxTotalAttempts, responseTimeoutMillisForEachAttempt);
if (hedgingDelayMillis >= 0) {
return new RetryConfig<>(retryRule, maxTotalAttempts, responseTimeoutMillisForEachAttempt,
hedgingDelayMillis);
} else {
return new RetryConfig<>(retryRule, maxTotalAttempts, responseTimeoutMillisForEachAttempt);
}
}
assert retryRuleWithContent != null;
return new RetryConfig<>(
retryRuleWithContent,
maxContentLength,
maxTotalAttempts,
responseTimeoutMillisForEachAttempt);

if (hedgingDelayMillis >= 0) {
return new RetryConfig<>(retryRuleWithContent, maxContentLength, maxTotalAttempts,
responseTimeoutMillisForEachAttempt, hedgingDelayMillis);
} else {
return new RetryConfig<>(retryRuleWithContent, maxContentLength, maxTotalAttempts,
responseTimeoutMillisForEachAttempt);
}
}

@Override
Expand All @@ -139,6 +175,7 @@
.add("retryRuleWithContent", retryRuleWithContent)
.add("maxTotalAttempts", maxTotalAttempts)
.add("responseTimeoutMillisForEachAttempt", responseTimeoutMillisForEachAttempt)
.add("hedgingDelayMillis", hedgingDelayMillis)

Check warning on line 178 in core/src/main/java/com/linecorp/armeria/client/retry/RetryConfigBuilder.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/com/linecorp/armeria/client/retry/RetryConfigBuilder.java#L178

Added line #L178 was not covered by tests
.add("maxContentLength", maxContentLength);
}
}
Loading
Loading