fix: retry consecutive retryable errors in sql stream

Retry consecutive retryable errors for the same resume token in a stream of PartialResultSets. This will prevent SpannerDataReader from throwing Unavailable exceptions while reading data. The ExecuteSqlStreaming RPC will be retried at most 115 times for the same resume token. This value is chosen as it will cause retries to be attempted for roughly the same amount of time as the default timeout of ExecuteStreamingSql (1 hour). Fixes #5977
googleapis · Mar 10, 2021 · fa5641d · fa5641d
1 parent 7ee3de9
commit fa5641d
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 4 deletions.
diff --git a/apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data.Tests/V1/SqlResultStreamTests.cs b/apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data.Tests/V1/SqlResultStreamTests.cs
@@ -108,10 +108,23 @@ public async Task MultipleErrors_ForSameResumeToken()
             // We expect calls of:
             // - Start: no token, MoveNext(yes), MoveNext(throw)
             // - Start: token1, MoveNext(throw)
-            // - No further calls, due to second failure
-            // Only the result with token1 is returned
+            // - Start: token1, succeeds until the end of the stream.
+            await AssertResultsAsync(stream, results);
+            Assert.Equal(3, client.Calls);
+        }
+
+        [Fact]
+        public async Task MultipleErrors_ForSameResumeToken_DoesNotRetryForever()
+        {
+            var results = CreateResultSets("token1", "token2");
+            var client = new FakeSpannerClient(results, CreateExceptionFilter("token2", RetriableStatusCode, 120));
+            var stream = CreateStream(client);
+
+            // Expected calls:
+            // - Start: no token, MoveNext(yes), MoveNext(throw)
+            // - Start: token1, MoveNext(throw) until 115 attempts have been made.
             await AssertResultsThenExceptionAsync(stream, results, 1);
-            Assert.Equal(2, client.Calls);
+            Assert.Equal(116, client.Calls);
         }
 
         [Fact]

diff --git a/apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/SqlResultStream.cs b/apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/SqlResultStream.cs
@@ -111,7 +111,9 @@ public async Task<bool> MoveNext(CancellationToken cancellationToken)
         private async Task<PartialResultSet> ComputeNextAsync(CancellationToken cancellationToken)
         {
             // The retry state is local to the method as we're not trying to handle callers retrying.
-            RetryState retryState = new RetryState(_client.Settings.Scheduler ?? SystemScheduler.Instance, _retrySettings);
+            // Max 115 consecutive errors is roughly equal to the default timeout of ExecuteStreamingSql (1 hour)
+            // based on the default backoff settings (InitialBackoff: 1 second, MaxBackoff: 32 seconds).
+            RetryState retryState = new RetryState(_client.Settings.Scheduler ?? SystemScheduler.Instance, _retrySettings, maxConsecutiveErrors: 115);
 
             while (true)
             {