Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
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
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 @@ public RetryConfigBuilder<T> maxTotalAttempts(int maxTotalAttempts) {
return this;
}

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

/**
* 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 RetryConfigBuilder<T> responseTimeoutForEachAttempt(Duration responseTime
*/
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 @@ ToStringHelper toStringHelper() {
.add("retryRuleWithContent", retryRuleWithContent)
.add("maxTotalAttempts", maxTotalAttempts)
.add("responseTimeoutMillisForEachAttempt", responseTimeoutMillisForEachAttempt)
.add("maxContentLength", maxContentLength);
.add("maxContentLength", maxContentLength)
.add("hedgingDelayMillis", hedgingDelayMillis);
}
}
Loading
Loading