Spanner: gRPC Retry and Timeout Settings should be used from configuration #20

Activity

1. kolea2

   assigned

   olavloite

   on Jun 24, 2019
2. olavloite commented on Jun 25, 2019

   

   olavloite

   on Jun 25, 2019

   Contributor

   More actions

   The Java client library for Spanner has been changed to use the gapic client and rely on Gax for almost all retries, so these constants are not used very much anymore. There are two exceptions:

   • Retrying aborted transactions: This is currently using the SpannerRetryHelper.
   • Retrying broken streams for result sets: This is currently using the constant referenced in this issue here: https://github.com/googleapis/google-cloud-java/blob/03f4bec198366b84516d991d99df85715293d7c5/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java#L918

   The retry settings from gapic are defined for the different gRPC methods. The two exceptions above are not directly related to one method, but could be caused by different methods, so there is no direct choice for which retry setting from the config that should be used. My suggestion would be:

   • Retrying transactions: Use the 'default' settings https://github.com/googleapis/google-cloud-java/blob/03f4bec198366b84516d991d99df85715293d7c5/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/v1/stub/SpannerStubSettings.java#L394
   • Retrying broken streams: Use the 'streaming' settings https://github.com/googleapis/google-cloud-java/blob/03f4bec198366b84516d991d99df85715293d7c5/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/v1/stub/SpannerStubSettings.java#L405

   @kolea2 and @crwilcox Do you both agree with that?
3. chingor13

   transferred this issue fromgoogleapis/google-cloud-javaon Jan 7, 2020
4. chingor13

   added

   type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.‘Nice-to-have’ improvement, new feature or different behavior or design.

   on Jan 7, 2020
5. googleapis

   locked and limited conversation to collaborators on Jan 8, 2020
6. googleapis

   unlocked this conversation on Jan 8, 2020
7. skuruppu commented on Jan 9, 2020

   

   skuruppu

   on Jan 9, 2020

   Contributor

   More actions

   The Java client library for Spanner has been changed to use the gapic client and rely on Gax for almost all retries, so these constants are not used very much anymore. There are two exceptions:

   • Retrying aborted transactions: This is currently using the SpannerRetryHelper.
   • Retrying broken streams for result sets: This is currently using the constant referenced in this issue here: https://github.com/googleapis/google-cloud-java/blob/03f4bec198366b84516d991d99df85715293d7c5/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java#L918

   The retry settings from gapic are defined for the different gRPC methods. The two exceptions above are not directly related to one method, but could be caused by different methods, so there is no direct choice for which retry setting from the config that should be used. My suggestion would be:

   • Retrying transactions: Use the 'default' settings https://github.com/googleapis/google-cloud-java/blob/03f4bec198366b84516d991d99df85715293d7c5/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/v1/stub/SpannerStubSettings.java#L394
   • Retrying broken streams: Use the 'streaming' settings https://github.com/googleapis/google-cloud-java/blob/03f4bec198366b84516d991d99df85715293d7c5/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/v1/stub/SpannerStubSettings.java#L405

   @kolea2 and @crwilcox Do you both agree with that?

   I think this is reasonable @olavloite. Please go ahead with making the necessary changes.
8. gcf-label-sync

   added

   api: spannerIssues related to the googleapis/java-spanner API.Issues related to the googleapis/java-spanner API.

   on Jan 29, 2020
9. olavloite commented on Feb 28, 2020

   

   olavloite

   on Feb 28, 2020

   Contributor

   More actions

   All custom retry values have now been replaced by values from the gapic configuration by #48 and #49.
10. olavloite

    closed this as completedon Feb 28, 2020