mpl(bigtable): error when read_buffer is not empty and stream finishes (#15775)

Author: scotthart

google/cloud/bigtable/ci/run_conformance_tests_proxy_bazel.sh

@@ -50,8 +50,8 @@ exit_status=$?
 # Run all the ExecuteQuery tests that either work or we plan to skip such as
 # CloseClient
 go test -v \
-  -run "TestExecuteQuery|TestExecuteQuery_PlanRefresh$|TestExecuteQuery_PlanRefresh_WithMetadataChange|TestExecuteQuery_PlanRefresh_Retries|TestExecuteQuery_PlanRefresh_RecoversAfterPermanentError" \
-  -skip "CloseClient|FailsOnEmptyMetadata|FailsOnExecuteQueryMetadata|FailsOnInvalidType|FailsOnNotEnoughData|FailsOnNotEnoughDataWithCompleteRows|FailsOnSuccesfulStreamWithNoToken|ChecksumMismatch|FailsOnTypeMismatch|FailsOnTypeMismatchWithinMap|FailsOnTypeMismatchWithinArray|FailsOnTypeMismatchWithinStruct|FailsOnStructMissingField|TestExecuteQuery_PlanRefresh_AfterResumeTokenCausesError|TestExecuteQuery_RetryTest_WithPlanRefresh|TestExecuteQuery_PlanRefresh_RespectsDeadline" \
+  -run "TestExecuteQuery|TestExecuteQuery_PlanRefresh$|TestExecuteQuery_PlanRefresh_WithMetadataChange|TestExecuteQuery_PlanRefresh_Retries|TestExecuteQuery_FailsOnSuccesfulStreamWithNoToken" \
+  -skip "CloseClient|FailsOnEmptyMetadata|FailsOnExecuteQueryMetadata|FailsOnInvalidType|FailsOnNotEnoughData|FailsOnNotEnoughDataWithCompleteRows|FailsOnSuccesfulStreamWithNoToken|ChecksumMismatch|FailsOnTypeMismatch|FailsOnTypeMismatchWithinMap|FailsOnTypeMismatchWithinArray|FailsOnTypeMismatchWithinStruct|FailsOnStructMissingField|TestExecuteQuery_PlanRefresh_AfterResumeTokenCausesError|TestExecuteQuery_RetryTest_WithPlanRefresh|TestExecuteQuery_PlanRefresh_RespectsDeadline|TestExecuteQuery_PlanRefresh_RecoversAfterPermanentError" \
   -proxy_addr=:9999
 exit_status=$?
 
@@ -67,7 +67,7 @@ exit_status=$?
 
 # Stream reading tests b/461232110
 #go test -v \
-#  -run "FailsOnNotEnoughData|FailsOnNotEnoughDataWithCompleteRows|FailsOnSuccesfulStreamWithNoToken|ChecksumMismatch" \
+#  -run "FailsOnNotEnoughData|FailsOnNotEnoughDataWithCompleteRows|ChecksumMismatch" \
 #  -proxy_addr=:9999
 #exit_status=$?
 

google/cloud/bigtable/internal/partial_result_set_source.cc

@@ -141,10 +141,10 @@ Status PartialResultSetSource::ReadFromStream() {
     return ProcessDataFromStream(result_set.result);
   }
   state_ = State::kFinished;
-  // The buffered_rows_ is expected to be empty because the last successful
-  // read would have had a sentinel resume_token, causing
+  // buffered_rows_ and read_buffer_ are expected to be empty because the last
+  // successful read would have had a sentinel resume_token, causing
   // ProcessDataFromStream to commit them.
-  if (!buffered_rows_.empty()) {
+  if (!buffered_rows_.empty() || !read_buffer_.empty()) {
     return internal::InternalError("Stream ended with uncommitted rows.",
                                    GCP_ERROR_INFO());
   }
@@ -180,7 +180,8 @@ Status PartialResultSetSource::ProcessDataFromStream(
     } else {
       read_buffer_.clear();
       buffered_rows_.clear();
-      return internal::InternalError("Failed to parse ProtoRows from buffer");
+      return internal::InternalError("Failed to parse ProtoRows from buffer",
+                                     GCP_ERROR_INFO());
     }
   }
 

google/cloud/bigtable/internal/retry_traits.h

@@ -54,7 +54,8 @@ struct QueryPlanRefreshRetry {
   static bool IsTransientFailure(Status const& status) {
     auto const code = status.code();
     return code == StatusCode::kAborted || code == StatusCode::kUnavailable ||
-           code == StatusCode::kInternal || IsQueryPlanExpired(status);
+           google::cloud::internal::IsTransientInternalError(status) ||
+           IsQueryPlanExpired(status);
   }
   static bool IsPermanentFailure(Status const& status) {
     return !IsOk(status) && !IsTransientFailure(status);