Skip to content

docs: add sample for setting timeout and retry settings#3445

Merged
lesv merged 3 commits into
GoogleCloudPlatform:masterfrom
olavloite:set-custom-timeout
Aug 1, 2020
Merged

docs: add sample for setting timeout and retry settings#3445
lesv merged 3 commits into
GoogleCloudPlatform:masterfrom
olavloite:set-custom-timeout

Conversation

@olavloite
Copy link
Copy Markdown
Contributor

Adds an example for how to set custom timeout and retry settings for the Spanner client.

@olavloite olavloite requested review from a team and hengfengli July 31, 2020 11:05
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 31, 2020
Comment on lines +46 to +64
// Set custom timeout and retry settings for the ExecuteSql RPC.
builder
.getSpannerStubSettingsBuilder()
.executeSqlSettings()
// Configure which errors should be retried.
.setRetryableCodes(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE)
.setRetrySettings(
RetrySettings.newBuilder()
// Configure retry delay settings.
.setInitialRetryDelay(Duration.ofMillis(500))
.setMaxRetryDelay(Duration.ofSeconds(64))
.setRetryDelayMultiplier(1.5)

// Configure RPC and total timeout settings.
.setInitialRpcTimeout(Duration.ofSeconds(60))
.setMaxRpcTimeout(Duration.ofSeconds(60))
.setRpcTimeoutMultiplier(1.0)
.setTotalTimeout(Duration.ofSeconds(60))
.build());
Copy link
Copy Markdown
Contributor

@lesv lesv Jul 31, 2020

Choose a reason for hiding this comment

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

This just looks weird, it took me a while to understand what was going on. I'm not sure how you fix it, but, I wonder if the .setProjectId(projectId) shouldn't be the first thing in this group, or, if you should do all the options on the SpannerOptions.Builder builder = line(s).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The problem here is that the setRetryableCodes(..) and setRetrySettings(..) methods return a UnaryCallSettings.Builder instance, and not a SpannerOptions.Builder, and that cannot (easily) be changed as it is part of the generated gapic client. What we are really doing here is exposing a part of the generated gapic client (builder) for the end user to be able to specify timeout and retry settings. That means that it is not possible to create the SpannerOptions instance in one single chain when this option is used, but that it must have at least one separate chain that sets the custom retry and/or timeout settings.

Adding the .setProjectId(projectId) to this group is possible, but I don't feel that it makes it more readable. I added a comment to clarify why the retry settings are set in a separate chain.

Copy link
Copy Markdown
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

LGTM - but I have one question.

@lesv
Copy link
Copy Markdown
Contributor

lesv commented Jul 31, 2020

Odd - I changed the question before I sent the review and it sent the old question. Please see in the PR.

Copy link
Copy Markdown
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

That's why I ask questions, I wasn't expecting that answer. Good that you explain it.

@lesv lesv merged commit 3cf2283 into GoogleCloudPlatform:master Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants