fix: retry executeStreamingSql when error code is retryable #795
Conversation
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #795 +/- ##
==========================================
+ Coverage 74.61% 74.64% +0.02%
==========================================
Files 43 43
Lines 20243 20262 +19
Branches 547 550 +3
==========================================
+ Hits 15105 15124 +19
Misses 5133 5133
Partials 5 5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
With my limited knowledge of the spanner library, this seems like a good idea to me 😄 If it could protect our users from temporary outages.
It might be good to discuss this change with @alexander-fenster as well, who has more knowledge of the internal workings of gRPC/gax.
|
Just one question: since
should we talk to API team to reconsider this? |
That is certainly an option, but it is something that we in that case should do in sync with all client libraries. I know that the Java and Go client libraries also have custom implementations that retry UNAVAILABLE errors for the executeStreamingSql RPC, and I assume that it is also the case of other languages. |
The streaming call executeStreamingSql is not automatically retried by gax, as the gapic configuration for the call does not specify any error codes that should automatically be retried. Instead, the PartialResultStream is responsible for retrying these calls with the appropriate resume token. Until now, the call was only retried when a valid resume token had been seen for the stream, meaning that if the initial call failed with a retryable error code (e.g. UNAVAILABLE), the stream would fail with this error. This fix ensures that the call is also retried when the error occurs for the initial call or before the stream has returned a valid resume token. Fixes googleapis#620.
…is#795) fix: retry executeStreamingSql when error code is retryable The streaming call executeStreamingSql is not automatically retried by gax, as the gapic configuration for the call does not specify any error codes that should automatically be retried. Instead, the PartialResultStream is responsible for retrying these calls with the appropriate resume token. Until now, the call was only retried when a valid resume token had been seen for the stream, meaning that if the initial call failed with a retryable error code (e.g. UNAVAILABLE), the stream would fail with this error. This fix ensures that the call is also retried when the error occurs for the initial call or before the stream has returned a valid resume token. Fixes googleapis#620.
The streaming call executeStreamingSql is not automatically retried by gax, as the gapic configuration for the call does not specify any error codes that should automatically be retried. Instead, the PartialResultStream is responsible for retrying these calls with the appropriate resume token. Until now, the call was only retried when a valid resume token had been seen for the stream, meaning that if the initial call failed with a retryable error code (e.g. UNAVAILABLE), the
stream would fail with this error. This fix ensures that the call is also retried when the error occurs for the initial call or before the stream has returned a valid resume token.
See also
nodejs-spanner/test/spanner.ts
Line 199 in 09b5c56
Fixes #620.