Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Unary Callables Deadline values respect the TotalTimeout in RetrySettings #1603

Merged
merged 143 commits into from
May 8, 2023
Merged
Changes from 1 commit
Commits
Show all changes
143 commits
Select commit Hold shift + click to select a range
dca83d7
chore: Add retry test
lqiu96 Mar 31, 2023
45c0765
chore: Check the timeout for unary callables
lqiu96 Apr 3, 2023
7862762
chore: Fix tests
lqiu96 Apr 4, 2023
7f5622f
chore: Address code smell
lqiu96 Apr 4, 2023
94b5017
chore: Add tests for DEADLINE_EXCEEDED
lqiu96 Apr 4, 2023
608d44b
chore: Add tests for Server-side streaming
lqiu96 Apr 4, 2023
0c1782f
chore: Start the timeout after http request invocation
lqiu96 Apr 4, 2023
f75f67e
chore: Fix format issues
lqiu96 Apr 4, 2023
7c4c841
chore: Remove Instant calculation with System clock
lqiu96 Apr 5, 2023
3668093
Merge branch 'main' into main-showcase_retries
lqiu96 Apr 5, 2023
d440b8e
chore: Add ITRetry test cases
lqiu96 Apr 5, 2023
bbf7f4f
chore: Fix the Retry showcase test
lqiu96 Apr 6, 2023
bee8c0e
chore: Fix the Retry showcase test
lqiu96 Apr 6, 2023
73882a7
chore: Convert duration to nanos
lqiu96 Apr 6, 2023
b0d83b9
chore: Set the readTimeout min to 20s
lqiu96 Apr 6, 2023
fc5cf13
chore: Add sucessful retry tests cases
lqiu96 Apr 6, 2023
2f1bc8a
chore: Add comments to timeout
lqiu96 Apr 6, 2023
ead0eb9
chore: Update the connect timeout
lqiu96 Apr 6, 2023
ca0770b
chore: Refactor timeout logic
lqiu96 Apr 7, 2023
21ed291
chore: Fix format issues
lqiu96 Apr 10, 2023
bb39513
chore: Add logic for deadlineScheduler
lqiu96 Apr 13, 2023
fab86be
chore: Fix format issues
lqiu96 Apr 13, 2023
95ef1a1
chore: Update logic
lqiu96 Apr 14, 2023
25190ec
chore: Update comment
lqiu96 Apr 14, 2023
f59de69
Merge branch 'main' into main-showcase_retries
lqiu96 Apr 14, 2023
133af94
chore: Update showcase test
lqiu96 Apr 14, 2023
ff5108f
chore: Fix format issues
lqiu96 Apr 14, 2023
e800373
chore: Fix logic
lqiu96 Apr 14, 2023
d757c05
Merge branch 'main' into main-showcase_retries
lqiu96 Apr 14, 2023
f139797
chore: Do not disconnect the connection
lqiu96 Apr 14, 2023
e1d12c5
chore: Disconnect after end
lqiu96 Apr 14, 2023
8ee7513
chore: Resolve steam close error
lqiu96 Apr 14, 2023
bed960e
chore: Fix disconnect logic
lqiu96 Apr 14, 2023
5c5eaec
chore: Fix disconnect logic
lqiu96 Apr 14, 2023
ab93117
chore: Update CI
lqiu96 Apr 14, 2023
f3d19b6
chore: Fix native test
lqiu96 Apr 14, 2023
1e38e5f
chore: Revert changes
lqiu96 Apr 17, 2023
5a10f4c
chore: try with rpc timeout 100ms
lqiu96 Apr 17, 2023
9b877ac
chore: Fix format issues
lqiu96 Apr 17, 2023
eff3513
chore: Re-run delivery loop with deadlineschedule priority
lqiu96 Apr 17, 2023
1d1dffa
chore: Check for timeoutExceeded
lqiu96 Apr 17, 2023
887f3bb
chore: Do not send message is time exceeded
lqiu96 Apr 17, 2023
102ab99
chore: Fix format issues
lqiu96 Apr 17, 2023
3f4b9a5
chore: Add timeout for tests
lqiu96 Apr 17, 2023
dffe194
chore: Fix format issues
lqiu96 Apr 17, 2023
47c8da6
chore: Refactor trailer logic
lqiu96 Apr 17, 2023
e5ccddd
chore: Refactor trailer logic
lqiu96 Apr 17, 2023
fdf6c63
chore: Rename variables
lqiu96 Apr 17, 2023
64b2c77
chore: Increase the wait to 1s
lqiu96 Apr 17, 2023
c5c2f54
chore: Fix format issues
lqiu96 Apr 17, 2023
d0893ea
chore: Set closed var as volatile
lqiu96 Apr 17, 2023
1281ab5
chore: Update logic for onClose
lqiu96 Apr 17, 2023
1ae5d3d
chore: Attempt with longer timeout
lqiu96 Apr 17, 2023
36e3788
chore: Empty commit
lqiu96 Apr 17, 2023
452fc97
chore: Fix format issues
lqiu96 Apr 17, 2023
0ad9442
chore: Trigger deliver loop instead of notifyListeners
lqiu96 Apr 17, 2023
0ad7a4d
chore: Remove variable
lqiu96 Apr 17, 2023
a769ee1
chore: Remove variable
lqiu96 Apr 17, 2023
65e9e67
chore: Fix close logic
lqiu96 Apr 17, 2023
0b926d5
chore: Revert graalvm ci
lqiu96 Apr 18, 2023
6b70a50
chore: Use 2s as delay
lqiu96 Apr 18, 2023
9af022a
chore: Update to 5s delay
lqiu96 Apr 18, 2023
66c757c
Merge branch 'main' into main-showcase_retries
lqiu96 Apr 18, 2023
fc59f98
chore: Add comments for timeout method
lqiu96 Apr 18, 2023
b773f89
chore: Use deliver loop in timeout
lqiu96 Apr 18, 2023
b571b6e
chore: Run matrix jobs sequentially
lqiu96 Apr 18, 2023
f97e7c0
chore: Fix format issues
lqiu96 Apr 18, 2023
1aec63c
chore: Fix format issues
lqiu96 Apr 18, 2023
de5927d
chore: Increase the wait to 10s
lqiu96 Apr 18, 2023
6a337c5
chore: Use 110ms delay
lqiu96 Apr 19, 2023
8be44c8
chore: Set delay to be 30s
lqiu96 Apr 19, 2023
42dd7ed
chore: Fix format issues
lqiu96 Apr 19, 2023
1de756c
chore: Log the onClose message
lqiu96 Apr 19, 2023
8756e27
chore: Remove localRunnable
lqiu96 Apr 19, 2023
51df7ea
chore: Fix format issues
lqiu96 Apr 19, 2023
a9ff512
chore: Lower the retry amounts
lqiu96 Apr 19, 2023
8f1627e
chore: Lower the retry amounts
lqiu96 Apr 19, 2023
becbc25
chore: Fix shouldRetry logic
lqiu96 Apr 19, 2023
c1f609f
chore: Log results of shouldRetry
lqiu96 Apr 19, 2023
be1f9fb
chore: Ignore other retry tests
lqiu96 Apr 19, 2023
490ccc1
chore: Add more logging
lqiu96 Apr 19, 2023
3a9bdfd
chore: Fix shouldRetry logic
lqiu96 Apr 19, 2023
87a41dc
chore: Remove small optimization
lqiu96 Apr 19, 2023
da26a56
chore: Temp ignore tests
lqiu96 Apr 19, 2023
0978549
chore: Temp ignore tests
lqiu96 Apr 19, 2023
93c8fdc
chore: Add more logging
lqiu96 Apr 19, 2023
2a9d2df
chore: revert back to checking for negative duration
lqiu96 Apr 19, 2023
a517d95
chore: Revert ignored test
lqiu96 Apr 19, 2023
0f8bbc8
chore: Fix logging
lqiu96 Apr 19, 2023
c1f914f
chore: Log timeout
lqiu96 Apr 19, 2023
6bb2d04
chore: Set min RPC timeout to be 1ms
lqiu96 Apr 19, 2023
3da7937
chore: Update the retry algorithms
lqiu96 Apr 19, 2023
d28b87f
chore: Clean up the algoritms
lqiu96 Apr 19, 2023
7c316a1
chore: Uncomment out ITRetry tests
lqiu96 Apr 19, 2023
ae3c2ec
chore: Refactor the retryAlgorithms
lqiu96 Apr 19, 2023
3fb78f7
chore: Add more comments
lqiu96 Apr 19, 2023
8cd3e3b
chore: Add in the parallel execution for ITs
lqiu96 Apr 19, 2023
b8704ff
chore: Add LRO showcase tests
lqiu96 Apr 20, 2023
eb6635a
chore: Fix format
lqiu96 Apr 20, 2023
0b76faf
chore: Remove deadline getters
lqiu96 Apr 20, 2023
acc3ee1
chore: Remove sonar changes
lqiu96 Apr 20, 2023
8b6445e
chore: Fix algorithm test
lqiu96 Apr 20, 2023
d458167
chore: Log the flaky test
lqiu96 Apr 20, 2023
09e7ff2
chore: Fix format
lqiu96 Apr 20, 2023
c20ee95
chore: Check for rpcTimeout being zero or negative
lqiu96 Apr 20, 2023
6552dbe
chore: Fix tests
lqiu96 Apr 20, 2023
0454e02
chore: Fix format issues
lqiu96 Apr 20, 2023
a1dcfdd
chore: Remove unused code
lqiu96 Apr 20, 2023
f9afed4
chore: Update comment for RetryAlgorithm
lqiu96 Apr 20, 2023
1d5c00a
chore: Fix format issues
lqiu96 Apr 20, 2023
a7983d3
chore: Use millis for timeout
lqiu96 Apr 21, 2023
15448d2
chore: Await termination for clients
lqiu96 Apr 21, 2023
924f7a0
chore: Fix format issues
lqiu96 Apr 21, 2023
f4ba8af
chore: Update LRO first call timeout value
lqiu96 Apr 21, 2023
a305123
chore: Update LRO test names
lqiu96 Apr 21, 2023
6e35172
chore: Remove the parallel showcase tests
lqiu96 Apr 21, 2023
d4bc792
chore: Add showcase sequence tests for retries
lqiu96 Apr 21, 2023
ccdf1e2
chore: Add showcase sequence tests for retries
lqiu96 Apr 21, 2023
561ffa3
chore: Update retry test name
lqiu96 Apr 24, 2023
3607a30
chore: Fix typos
lqiu96 Apr 24, 2023
cb34160
chore: Fix server streaming callable test
lqiu96 Apr 25, 2023
cf5ba07
chore: Clean up tests
lqiu96 Apr 25, 2023
07ecd7a
chore: Remove retry tests
lqiu96 Apr 25, 2023
b4812e3
chore: Fix format issues
lqiu96 Apr 25, 2023
ed6b53c
chore: Address PR comments
lqiu96 May 1, 2023
2095745
chore: Update java.time.Duration import
lqiu96 May 1, 2023
a8e3153
chore: Update values for LRO showcase test
lqiu96 May 1, 2023
760f5fe
chore: Update values for LRO showcase test
lqiu96 May 1, 2023
d890dee
chore: Fix format issues
lqiu96 May 1, 2023
3086821
chore: Update variable name
lqiu96 May 3, 2023
084b592
chore: Convert shouldRetry logic to use milliseconds
lqiu96 May 3, 2023
145d084
chore: Remove jitter from tests
lqiu96 May 3, 2023
e9c1645
chore: Fix showcase test
lqiu96 May 3, 2023
600a2dc
chore: Log the attempt callable timeout
lqiu96 May 3, 2023
5d81e85
chore: Update LRO test case
lqiu96 May 3, 2023
c429d96
chore: Add logging
lqiu96 May 3, 2023
5527700
chore: Use millis for timeout
lqiu96 May 4, 2023
13881bb
chore: Address PR comments
lqiu96 May 5, 2023
c052a87
chore: Update to use TestClientInitializer class
lqiu96 May 5, 2023
069acc6
Merge branch 'main' into main-showcase_retries
lqiu96 May 5, 2023
6cc15fb
chore: Fix client initialize method names
lqiu96 May 5, 2023
e099ac1
chore: Add back public method
lqiu96 May 8, 2023
0a3cadf
chore: Add back public methods
lqiu96 May 8, 2023
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
Prev Previous commit
Next Next commit
chore: Clean up the algoritms
lqiu96 committed Apr 19, 2023
commit d28b87f36e4d1b294cf65ccebf0344e2c7867e87
78 changes: 39 additions & 39 deletions .github/workflows/sonar.yaml
Original file line number Diff line number Diff line change
@@ -44,42 +44,42 @@ jobs:
tar -xf showcase-*
./gapic-showcase run &
cd -
# - name: Build and analyze for full test coverage
# env:
# GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
# SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
# run: |
# mvn -B verify -Dcheckstyle.skip \
# -DenableFullTestCoverage \
# -Penable-integration-tests \
# org.sonarsource.scanner.maven:sonar-maven-plugin:sonar \
# -Dsonar.projectKey=googleapis_gapic-generator-java \
# -Dsonar.organization=googleapis \
# -Dsonar.host.url=https://sonarcloud.io
#
# - name: Build and analyze Showcase Integration Tests Coverage
# env:
# GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
# SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
# run: |
# mvn -B clean verify -Dcheckstyle.skip \
# -DskipUnitTests \
# -Penable-integration-tests \
# -DenableShowcaseTestCoverage \
# org.sonarsource.scanner.maven:sonar-maven-plugin:sonar \
# -Dsonar.projectKey=googleapis_gapic-generator-java_integration_tests \
# -Dsonar.organization=googleapis \
# -Dsonar.host.url=https://sonarcloud.io \
# -Dsonar.projectName=java_showcase_integration_tests
# - name: Build and analyze Showcase Unit Tests Coverage
# env:
# GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
# SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
# run: |
# mvn -B clean test -Dcheckstyle.skip \
# -DenableShowcaseTestCoverage \
# org.sonarsource.scanner.maven:sonar-maven-plugin:sonar \
# -Dsonar.projectKey=googleapis_gapic-generator-java_unit_tests \
# -Dsonar.organization=googleapis \
# -Dsonar.host.url=https://sonarcloud.io \
# -Dsonar.projectName=java_showcase_unit_tests
- name: Build and analyze for full test coverage
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
run: |
mvn -B verify -Dcheckstyle.skip \
-DenableFullTestCoverage \
-Penable-integration-tests \
org.sonarsource.scanner.maven:sonar-maven-plugin:sonar \
-Dsonar.projectKey=googleapis_gapic-generator-java \
-Dsonar.organization=googleapis \
-Dsonar.host.url=https://sonarcloud.io

- name: Build and analyze Showcase Integration Tests Coverage
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
run: |
mvn -B clean verify -Dcheckstyle.skip \
-DskipUnitTests \
-Penable-integration-tests \
-DenableShowcaseTestCoverage \
org.sonarsource.scanner.maven:sonar-maven-plugin:sonar \
-Dsonar.projectKey=googleapis_gapic-generator-java_integration_tests \
-Dsonar.organization=googleapis \
-Dsonar.host.url=https://sonarcloud.io \
-Dsonar.projectName=java_showcase_integration_tests
- name: Build and analyze Showcase Unit Tests Coverage
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
run: |
mvn -B clean test -Dcheckstyle.skip \
-DenableShowcaseTestCoverage \
org.sonarsource.scanner.maven:sonar-maven-plugin:sonar \
-Dsonar.projectKey=googleapis_gapic-generator-java_unit_tests \
-Dsonar.organization=googleapis \
-Dsonar.host.url=https://sonarcloud.io \
-Dsonar.projectName=java_showcase_unit_tests
Original file line number Diff line number Diff line change
@@ -176,7 +176,6 @@ public void start(Listener<ResponseT> responseListener, HttpJsonMetadata request
// If the future timeout amount is guaranteed to not be a negative value.
// The RetryAlgorithm checks that the timeout is not negative.
long timeoutNanos = timeout.toNanos();
System.out.println("Timeout Nanos: " + timeoutNanos);
this.deadlineCancellationExecutor.schedule(this::timeout, timeoutNanos, TimeUnit.NANOSECONDS);
}
}
Original file line number Diff line number Diff line change
@@ -56,8 +56,6 @@ public static <RequestT, ResponseT> HttpJsonClientCall<RequestT, ResponseT> newC
HttpJsonCallOptions callOptions = httpJsonContext.getCallOptions();
// HttpJsonChannel expects the HttpJsonCallOptions and we store the timeout duration
// inside the HttpJsonCallOptions
System.out.println("Context Timeout: " + httpJsonContext.getTimeout());
System.out.println("Call Options Timeout: " + callOptions.getTimeout());
if (callOptions.getTimeout() == null
|| httpJsonContext.getTimeout().compareTo(callOptions.getTimeout()) < 0) {
callOptions = callOptions.toBuilder().setTimeout(httpJsonContext.getTimeout()).build();
Original file line number Diff line number Diff line change
@@ -167,12 +167,10 @@ void handleAttempt(Throwable throwable, ResponseT response) {
return;
}

System.out.println("Throwable: " + throwable + " Response: " + response);
TimedAttemptSettings nextAttemptSettings =
retryAlgorithm.createNextAttempt(retryingContext, throwable, response, attemptSettings);
boolean shouldRetry =
retryAlgorithm.shouldRetry(retryingContext, throwable, response, nextAttemptSettings);
System.out.println("RPC Timeout: " + nextAttemptSettings + " Should Retry: " + shouldRetry);
if (shouldRetry) {
// Log retry info
if (LOG.isLoggable(Level.FINEST)) {
Original file line number Diff line number Diff line change
@@ -204,6 +204,9 @@ public boolean shouldRetry(TimedAttemptSettings nextAttemptSettings) {
return false;
}

// For RPCs, do not attempt to retry if the timeout has either passed (negative)
// or will pass immediately (zero). For any positive timeout value, the
// deadlineScheduler will terminate in the future (even if the timeout is small).
Duration rpcTimeout = nextAttemptSettings.getRpcTimeout();
if (totalTimeout > 0 && (rpcTimeout.isNegative() || rpcTimeout.isZero())) {
return false;
Original file line number Diff line number Diff line change
@@ -156,7 +156,6 @@ public TimedAttemptSettings createNextAttempt(
// a small optimization that avoids calling relatively heavy methods
// like timedAlgorithm.createNextAttempt(), when it is not necessary.
if (!shouldRetryBasedOnResult(context, previousThrowable, previousResponse)) {
System.out.println("Not retrying because of result: " + previousThrowable);
return null;
}

Original file line number Diff line number Diff line change
@@ -36,13 +36,11 @@

import com.google.api.gax.core.FakeApiClock;
import com.google.api.gax.rpc.testing.FakeCallContext;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.threeten.bp.Duration;

@Ignore
@RunWith(JUnit4.class)
public class ExponentialRetryAlgorithmTest {
private final FakeApiClock clock = new FakeApiClock(0L);
@@ -174,15 +172,25 @@ public void testShouldRetryFalseOnMaxAttempts() {
assertFalse(algorithm.shouldRetry(attempt));
}

// First attempt runs at 0ms
// Second attempt runs at 60ms if shouldRetry is true
// - RPC timeout is 2ms and Time Left is 140ms (shouldRetry == true)
// Third attempt runs at 60ms if shouldRetry is true
// - RPC timeout is 4ms and Time Left is 120ms (shouldRetry == true)
// Fourth attempt runs at 60ms if shouldRetry is true
// - RPC timeout is 8ms and Time Left is 20ms (shouldRetry == true)
// Fifth attempt runs at 60ms if shouldRetry is true
// - RPC timeout is 8ms and Time Left is -40ms (shouldRetry == false)
@Test
public void testShouldRetryFalseOnMaxTimeout() {
// Simulate each attempt with 60ms of clock time.
// "attempt" = RPC Timeout + createNextAttempt() and shouldRetry()
TimedAttemptSettings attempt = algorithm.createFirstAttempt();
for (int i = 0; i < 4; i++) {
clock.incrementNanoTime(Duration.ofMillis(60L).toNanos());
assertTrue(algorithm.shouldRetry(attempt));
attempt = algorithm.createNextAttempt(attempt);
clock.incrementNanoTime(Duration.ofMillis(60L).toNanos());
}

assertFalse(algorithm.shouldRetry(attempt));
}
}