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.

Sign up for GitHub

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

Jump to bottom

docs: improve timeout and retry sample #2630

Merged
merged 4 commits into from Sep 28, 2023
Merged

docs: improve timeout and retry sample #2630

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f66d91
docs: improve timeout and retry sample
olavloite Sep 17, 2023
566dacf
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Sep 17, 2023
8bb2fa7
chore: update timeout values based on what is possible with other lan…
olavloite Sep 18, 2023
21c70bb
test: move sample test to separate class
olavloite Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Expand Up @@ -50,7 +50,7 @@ If you are using Maven without the BOM, add this to your dependencies:
If you are using Gradle 5.x or later, add this to your dependencies:

```Groovy
implementation platform('com.google.cloud:libraries-bom:26.22.0')
implementation platform('com.google.cloud:libraries-bom:26.23.0')

implementation 'com.google.cloud:google-cloud-spanner'
```
Expand Down
42 changes: 42 additions & 0 deletions google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java
View file
Open in desktop
Expand Up @@ -3419,6 +3419,48 @@ public void testRetryOnResourceExhausted() {
}
}

@Test
public void testSampleRetrySettings() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we carve it into a separate test class similar to what we do for most other samples code? It improves the discoverability of the test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally only do that for the integration tests that actually execute the sample code. See for example https://github.com/googleapis/java-spanner/blob/main/samples/snippets/src/test/java/com/example/spanner/DmlReturningSampleIT.java. This test does not run the actual sample code, as we don't have any infrastructure in place for running the samples against an in-mem mock server. Instead, it only verifies that not setting DEADLINE_EXCEEDED as one of the retryable codes will cause the operation to fail with DEADLINE_EXCEEDED.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Consider adding the new test in a separate class. google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java is already overloaded for testing too many things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the sample test to a separate class (and created an abstract base class for creating mock server tests, so it's easier to set up more of those test classes).

String sql =
"INSERT INTO Singers (SingerId, FirstName, LastName)\n"
+ "VALUES (20, 'George', 'Washington')";
mockSpanner.putStatementResult(StatementResult.update(Statement.of(sql), 1L));

SpannerOptions.Builder builder =
SpannerOptions.newBuilder()
.setProjectId("p")
.setCredentials(NoCredentials.getInstance())
.setChannelProvider(channelProvider);
// Set a timeout value for the ExecuteSql RPC that is so low that it will always be triggered.
// This should cause the RPC to fail with a DEADLINE_EXCEEDED error.
builder
.getSpannerStubSettingsBuilder()
.executeSqlSettings()
.setRetryableCodes(StatusCode.Code.UNAVAILABLE)
.setRetrySettings(
RetrySettings.newBuilder()
.setInitialRetryDelay(Duration.ofMillis(500))
.setMaxRetryDelay(Duration.ofSeconds(16))
.setRetryDelayMultiplier(1.5)
.setInitialRpcTimeout(Duration.ofNanos(1L))
.setMaxRpcTimeout(Duration.ofNanos(1L))
.setRpcTimeoutMultiplier(1.0)
.setTotalTimeout(Duration.ofNanos(1L))
.build());
// Create a Spanner client using the custom retry and timeout settings.
try (Spanner spanner = builder.build().getService()) {
DatabaseClient client = spanner.getDatabaseClient(DatabaseId.of("p", "i", "d"));
SpannerException exception =
assertThrows(
SpannerException.class,
() ->
client
.readWriteTransaction()
.run(transaction -> transaction.executeUpdate(Statement.of(sql))));
assertEquals(ErrorCode.DEADLINE_EXCEEDED, exception.getErrorCode());
}
}

static void assertAsString(String expected, ResultSet resultSet, int col) {
assertEquals(expected, resultSet.getValue(col).getAsString());
assertEquals(ImmutableList.of(expected), resultSet.getValue(col).getAsStringList());
Expand Down
4 changes: 2 additions & 2 deletions samples/snippets/src/main/java/com/example/spanner/CustomTimeoutAndRetrySettingsExample.java
View file
Open in desktop
Expand Up @@ -49,15 +49,15 @@ static void executeSqlWithCustomTimeoutAndRetrySettings(
.getSpannerStubSettingsBuilder()
.executeSqlSettings()
// Configure which errors should be retried.
.setRetryableCodes(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE)
.setRetryableCodes(Code.UNAVAILABLE)
.setRetrySettings(
RetrySettings.newBuilder()
// Configure retry delay settings.
// The initial amount of time to wait before retrying the request.
.setInitialRetryDelay(Duration.ofMillis(500))
// The maximum amount of time to wait before retrying. I.e. after this value is
// reached, the wait time will not increase further by the multiplier.
.setMaxRetryDelay(Duration.ofSeconds(64))
.setMaxRetryDelay(Duration.ofSeconds(16))
// The previous wait time is multiplied by this multiplier to come up with the next
// wait time, until the max is reached.
.setRetryDelayMultiplier(1.5)
Expand Down