allenpradeep commented on a change in pull request #12011:
URL: https://github.com/apache/beam/pull/12011#discussion_r441171212
##########
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java
##########
@@ -63,23 +65,19 @@ static SpannerAccessor create(SpannerConfig spannerConfig) {
ValueProvider<Duration> commitDeadline = spannerConfig.getCommitDeadline();
if (commitDeadline != null && commitDeadline.get().getMillis() > 0) {
- // In Spanner API version 1.21 or above, we can set the deadline / total
Timeout on an API
- // call using the following code:
- //
- // UnaryCallSettings.Builder commitSettings =
- // builder.getSpannerStubSettingsBuilder().commitSettings();
- // RetrySettings.Builder commitRetrySettings =
commitSettings.getRetrySettings().toBuilder()
- // commitSettings.setRetrySettings(
- // commitRetrySettings.setTotalTimeout(
- // Duration.ofMillis(getCommitDeadlineMillis().get()))
- // .build());
- //
- // However, at time of this commit, the Spanner API is at only at
v1.6.0, where the only
- // method to set a deadline is with GRPC Interceptors, so we have to use
that...
- SpannerInterceptorProvider interceptorProvider =
- SpannerInterceptorProvider.createDefault()
- .with(new
CommitDeadlineSettingInterceptor(commitDeadline.get()));
- builder.setInterceptorProvider(interceptorProvider);
+ // Set the GRPC deadline on the Commit API call.
+ UnaryCallSettings.Builder commitSettings =
+ builder.getSpannerStubSettingsBuilder().commitSettings();
+ RetrySettings.Builder commitRetrySettings =
commitSettings.getRetrySettings().toBuilder();
+ commitSettings.setRetrySettings(
+ commitRetrySettings
Review comment:
This patch sounds good. But i have a question. RetrySettings seems to
support retry with backoff. The following retry mechanism could be moved to
here as well.
https://github.com/apache/beam/blob/master/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java#L1485
Can that be done? Not as part of this patch. I could send out a PR as well
after testing. That would clean up some code in SpannerIO as well
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
