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

Spanner: Add specific timeout for Partitioned DML with default of 2 hours #5709

Conversation

olavloite
Copy link

Partitioned DML statements should use a different timeout value than the default timeout value used for normal queries and update statements, as these could be long running. This PR adds a default timeout of 2 hours for Partitioned DML statements and an extra configuration option for setting the timeout.

@olavloite olavloite requested a review from a team as a code owner July 9, 2019 09:14
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 9, 2019
@olavloite olavloite changed the title Spanner: Add specific timeout for partitioned dml with default of 2 hours Spanner: Add specific timeout for Partitioned DML with default of 2 hours Jul 9, 2019
@codecov
Copy link

codecov bot commented Jul 9, 2019

Codecov Report

Merging #5709 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5709   +/-   ##
=========================================
  Coverage     46.72%   46.72%           
  Complexity    24647    24647           
=========================================
  Files          2351     2351           
  Lines        256149   256149           
  Branches      29327    29327           
=========================================
  Hits         119688   119688           
  Misses       127539   127539           
  Partials       8922     8922

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4bba989...5f2bba8. Read the comment docs.

.setChannelProvider(channelProvider)
.setCredentials(NoCredentials.getInstance());
// Set normal DML timeout value.
builder.getSpannerStubSettingsBuilder().executeSqlSettings().setRetrySettings(retrySettings);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to add an assertion that the PDML timeout is 2 hrs?

@@ -114,6 +116,7 @@ private SpannerOptions(Builder builder) {
} catch (IOException e) {
throw SpannerExceptionFactory.newSpannerException(e);
}
partitionedDmlTimeout = builder.partitionedDmlTimeout;
Copy link
Contributor

Choose a reason for hiding this comment

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

very minor - I would move this above the try-catch, after sessionLabels.

@kolea2 kolea2 merged commit 3288c73 into googleapis:master Jul 10, 2019
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.

3 participants