Skip to content

Spanner: Use gax for retries - #4940

Merged
olavloite merged 26 commits into
googleapis:masterfrom
olavloite:spanner-use-gax-for-retries
May 29, 2019
Merged

Spanner: Use gax for retries#4940
olavloite merged 26 commits into
googleapis:masterfrom
olavloite:spanner-use-gax-for-retries

Conversation

@olavloite

@olavloiteolavloite commented Apr 13, 2019

Copy link
Copy Markdown
Contributor

Use GAX instead of the custom SpannerImpl.runWithRetries method for retrying server calls.

This migration involves three changes:

  • Remove the override of the generated default settings in GapicSpannerRpc that set the retryable error codes to an empty set. This will automatically enable GAX retries for all server calls using the default values that have already been defined. The defaults are the same as the defaults that are used by SpannerClient.
  • Add configuration options to SpannerOptions to allow the user to change the default RetrySettings. As SpannerOptions is used to create a Spanner instance, which again contains both a DatabaseClient, DatabaseAdminClient and InstanceAdminClient, SpannerOptions must contain configuration options for all three underlying clients.
  • The runWithRetries method in SpannerImpl is removed and all references to it changed into direct calls that will automatically be retried by GAX.

To avoid a lot of unnecessary configuration options on SpannerOptions, the retry settings of the three underlying clients are exposed by exposing the StubSettings.Builders that are used to create the underlying clients. This only adds three extra methods to SpannerOptions, while still granting (advanced) users access to all configuration options.

The client library will use the defaults that are globally defined for retries in the configuration yaml files of Spanner (i.e. in the files spanner_gapic.yaml, spanner_admin_database_gapic.yaml and spanner_admin_instance_gapic.yaml)

The following defaults are defined:

  • Spanner default settings:
    • Initial retry delay: 1 second
    • Retry multiplier: 1.3
    • Max retry delay: 32 seconds
    • Initial RPC timeout: 60 seconds
    • RPC timeout multiplier: 1.0
    • Max RPC timeout: 60 seconds
    • Total timeout: 600 seconds
  • Spanner streaming settings (used for executeStreamingSql and streamingRead):
    • Initial retry delay: 1 second
    • Retry multiplier: 1.3
    • Max retry delay: 32 seconds
    • Initial RPC timeout: 120 seconds
    • RPC timeout multiplier: 1.0
    • Max RPC timeout: 120 seconds
    • Total timeout: 1200 seconds
  • Spanner long-running settings (used for commit):
    • Initial retry delay: 1 second
    • Retry multiplier: 1.3
    • Max retry delay: 32 seconds
    • Initial RPC timeout: 3600 seconds
    • RPC timeout multiplier: 1.0
    • Max RPC timeout: 3600 seconds
    • Total timeout: 3600 seconds
  • Database admin default settings (used for all methods except methods that return an operation, i.e. createDatabase and updateDatabaseDdl, these have a total timeout of 24 hours):
    • Initial retry delay: 1 second
    • Retry multiplier: 1.3
    • Max retry delay: 32 seconds
    • Initial RPC timeout: 60 seconds
    • RPC timeout multiplier: 1.0
    • Max RPC timeout: 60 seconds
    • Total timeout: 600 seconds
  • Instance admin default settings (used for all methods except methods that return an operation, i.e. createInstance and updateInstance, these have a total timeout of 24 hours):
    • Initial retry delay: 1 second
    • Retry multiplier: 1.3
    • Max retry delay: 32 seconds
    • Initial RPC timeout: 60 seconds
    • RPC timeout multiplier: 1.0
    • Max RPC timeout: 60 seconds
    • Total timeout: 600 seconds

@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 13, 2019
@codecov

codecovBot commented Apr 14, 2019

Copy link
Copy Markdown

Codecov Report

Merging #4940 into master will increase coverage by 0.33%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #4940 +/- ##
============================================
+ Coverage 49.98% 50.31% +0.33% - Complexity 23668 23676 +8 
============================================
Files 2238 2238 Lines 226007 226059 +52 Branches 24191 24955 +764 ============================================
+ Hits 112974 113751 +777 - Misses 103693 103707 +14 + Partials 9340 8601 -739
Impacted FilesCoverage ΔComplexity Δ
...re/src/main/java/com/google/cloud/BaseService.java55.55% <0%> (-6.95%)2% <0%> (ø)
...le/cloud/compute/deprecated/DeprecationStatus.java88.63% <0%> (-3.13%)20% <0%> (ø)
...va/com/google/cloud/http/HttpTransportOptions.java56.33% <0%> (-1.64%)10% <0%> (ø)
...ge/contrib/nio/CloudStorageFileSystemProvider.java61.26% <0%> (-0.29%)70% <0%> (ø)
...ain/java/com/google/cloud/logging/LoggingImpl.java88.44% <0%> (-0.2%)73% <0%> (ø)
.../google/cloud/bigquery/spi/v2/HttpBigQueryRpc.java6.92% <0%> (-0.06%)2% <0%> (ø)
...om/google/cloud/storage/spi/v1/HttpStorageRpc.java1.82% <0%> (-0.01%)1% <0%> (ø)
...loud/compute/v1/InsertNodeTemplateHttpRequest.java23.92% <0%> (ø)9% <0%> (ø)⬇️
...in/java/com/google/cloud/compute/v1/NodeGroup.java38.6% <0%> (ø)17% <0%> (ø)⬇️
...java/com/google/cloud/compute/v1/NodeTemplate.java36.41% <0%> (ø)19% <0%> (ø)⬇️
... and 147 more

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 2eaa9f4...28935c3. Read the comment docs.

@olavloite
olavloiteforce-pushed the spanner-use-gax-for-retries branch from 93fa13d to 7fc30feCompareApril 16, 2019 12:33
@olavloiteolavloite changed the title [WIP] Spanner: Use gax for retriesSpanner: Use gax for retriesApr 17, 2019
@olavloite
olavloite marked this pull request as ready for review April 17, 2019 10:15
@olavloite
olavloite requested a review from a teamApril 17, 2019 10:15
@yoshi-automationyoshi-automation added the 🚨 critical P0 critical issue. Requires immediate fix label Apr 20, 2019
@olavloite
olavloiteforce-pushed the spanner-use-gax-for-retries branch from c5cfbd5 to 615a600CompareApril 22, 2019 16:48
@kolea2kolea2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 24, 2019
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 24, 2019
@kolea2kolea2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 1, 2019
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 1, 2019
@codecov-io

codecov-io commented May 1, 2019

Copy link
Copy Markdown

Codecov Report

Merging #4940 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #4940 +/- ##
============================================
+ Coverage 50.41% 50.42% +<.01% - Complexity 23796 23797 +1 
============================================
Files 2251 2251 Lines 226833 226833 Branches 24966 24966 ============================================
+ Hits 114368 114370 +2 + Misses 103858 103857 -1 + Partials 8607 8606 -1
Impacted FilesCoverage ΔComplexity Δ
...able/gaxx/reframing/ReframingResponseObserver.java90.82% <0%> (+1.83%)30% <0%> (+1%)⬆️

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 db3241b...1302c06. Read the comment docs.

@sduskissduskis added needs work This is a pull request that needs a little love. and removed 🚨 critical P0 critical issue. Requires immediate fix labels May 17, 2019
@olavloite
olavloiteforce-pushed the spanner-use-gax-for-retries branch from 129a2d0 to 000f3c3CompareMay 17, 2019 13:06
@olavloiteolavloite removed the needs work This is a pull request that needs a little love. label May 17, 2019
@yoshi-automationyoshi-automation added the 🚨 critical P0 critical issue. Requires immediate fix label May 17, 2019
@olavloite

Copy link
Copy Markdown
ContributorAuthor

This PR will also solve the original issue mentioned in #3616, as it will enable users to configure timeouts and custom retry settings for a SpannerOptions instance.

@olavloite
olavloite merged commit 2ae5ca0 into googleapis:masterMay 29, 2019
@olavloite
olavloite deleted the spanner-use-gax-for-retries branch May 29, 2019 06:12
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yesThis human has signed the Contributor License Agreement.🚨 criticalP0 critical issue. Requires immediate fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@olavloite@codecov-io@kolea2@sduskis@googlebot@yoshi-automation@yoshi-kokoro