From 4a1c7d8afa5ab6d455f4b6433609f8e03d0dc7c5 Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Mon, 17 Nov 2025 18:16:50 -0500 Subject: [PATCH 1/2] impl(bigtable): error when read_buffer is not empty and stream finishes --- .../bigtable/ci/run_conformance_tests_proxy_bazel.sh | 4 ++-- .../cloud/bigtable/internal/partial_result_set_source.cc | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/google/cloud/bigtable/ci/run_conformance_tests_proxy_bazel.sh b/google/cloud/bigtable/ci/run_conformance_tests_proxy_bazel.sh index 973e9688bde67..3234ecba262cf 100755 --- a/google/cloud/bigtable/ci/run_conformance_tests_proxy_bazel.sh +++ b/google/cloud/bigtable/ci/run_conformance_tests_proxy_bazel.sh @@ -50,7 +50,7 @@ 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" \ + -run "TestExecuteQuery|TestExecuteQuery_PlanRefresh$|TestExecuteQuery_PlanRefresh_WithMetadataChange|TestExecuteQuery_PlanRefresh_Retries|TestExecuteQuery_PlanRefresh_RecoversAfterPermanentError|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" \ -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=$? diff --git a/google/cloud/bigtable/internal/partial_result_set_source.cc b/google/cloud/bigtable/internal/partial_result_set_source.cc index ef74c7155f1fc..1c152a9b5c0ae 100644 --- a/google/cloud/bigtable/internal/partial_result_set_source.cc +++ b/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()); } } From 1e2a5671bbe5535f21b705f75335b4ab0c31689e Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Mon, 17 Nov 2025 19:24:12 -0500 Subject: [PATCH 2/2] add back TransientInternalError retry check --- google/cloud/bigtable/ci/run_conformance_tests_proxy_bazel.sh | 4 ++-- google/cloud/bigtable/internal/retry_traits.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/google/cloud/bigtable/ci/run_conformance_tests_proxy_bazel.sh b/google/cloud/bigtable/ci/run_conformance_tests_proxy_bazel.sh index 3234ecba262cf..cacca2022386f 100755 --- a/google/cloud/bigtable/ci/run_conformance_tests_proxy_bazel.sh +++ b/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|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" \ + -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=$? diff --git a/google/cloud/bigtable/internal/retry_traits.h b/google/cloud/bigtable/internal/retry_traits.h index 9f6034a6125a9..e22b75002f232 100644 --- a/google/cloud/bigtable/internal/retry_traits.h +++ b/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);