diff --git a/google/cloud/bigtable/internal/bigtable_stub_factory.cc b/google/cloud/bigtable/internal/bigtable_stub_factory.cc index c95d23a0db618..fe294cfd05f07 100644 --- a/google/cloud/bigtable/internal/bigtable_stub_factory.cc +++ b/google/cloud/bigtable/internal/bigtable_stub_factory.cc @@ -117,7 +117,13 @@ std::shared_ptr CreateBigtableStubRandomTwoLeastUsed( self->set_last_refresh_status(s); } if (!s.ok()) { - GCP_LOG(WARNING) << "Failed to refresh connection. Error: " << s; + if (ChannelUsage::IsSuccessfulRefreshStatus(s)) { + GCP_LOG(WARNING) + << "Connection refreshed; treating received Status as non-error: " + << s; + } else { + GCP_LOG(WARNING) << "Failed to refresh connection. Error: " << s; + } } }; diff --git a/google/cloud/bigtable/internal/channel_usage.h b/google/cloud/bigtable/internal/channel_usage.h index 2a2e6b583c4bc..93d6ac1c098f1 100644 --- a/google/cloud/bigtable/internal/channel_usage.h +++ b/google/cloud/bigtable/internal/channel_usage.h @@ -43,9 +43,16 @@ class ChannelUsage : public std::enable_shared_from_this> { outstanding_rpcs_(initial_outstanding_rpcs), last_refresh_status_(std::move(last_refresh_status)) {} + static bool IsSuccessfulRefreshStatus(Status const& s) { + // kPermissionDenied and kNotFound are considered acceptable as those may + // result from a permissions configuration issue and not an actual failure. + return s.ok() || s.code() == StatusCode::kPermissionDenied || + s.code() == StatusCode::kNotFound; + } + StatusOr instant_outstanding_rpcs() { std::scoped_lock lk(mu_); - if (!last_refresh_status_.ok()) return last_refresh_status_; + if (!IsRefreshSuccessful(lk)) return last_refresh_status_; return outstanding_rpcs_; } @@ -78,6 +85,10 @@ class ChannelUsage : public std::enable_shared_from_this> { } private: + bool IsRefreshSuccessful(std::scoped_lock const&) const { + return IsSuccessfulRefreshStatus(last_refresh_status_); + } + mutable std::mutex mu_; std::shared_ptr stub_; int outstanding_rpcs_ = 0; diff --git a/google/cloud/bigtable/internal/channel_usage_test.cc b/google/cloud/bigtable/internal/channel_usage_test.cc index baeccf1119198..4203377f3367f 100644 --- a/google/cloud/bigtable/internal/channel_usage_test.cc +++ b/google/cloud/bigtable/internal/channel_usage_test.cc @@ -63,12 +63,18 @@ TEST(ChannelUsageTest, InstantOutstandingRpcs) { TEST(ChannelUsageTest, SetLastRefreshStatus) { auto mock = std::make_shared(); auto channel = std::make_shared>(mock); - Status expected_status = internal::InternalError("uh oh"); + Status error_status = internal::InternalError("uh oh"); + Status not_found_status = internal::NotFoundError("not found"); + Status permission_denied_status = internal::PermissionDeniedError("denied"); (void)channel->AcquireStub(); EXPECT_THAT(channel->instant_outstanding_rpcs(), IsOkAndHolds(1)); - channel->set_last_refresh_status(expected_status); + channel->set_last_refresh_status(not_found_status); + EXPECT_THAT(channel->instant_outstanding_rpcs(), IsOkAndHolds(1)); + channel->set_last_refresh_status(permission_denied_status); + EXPECT_THAT(channel->instant_outstanding_rpcs(), IsOkAndHolds(1)); + channel->set_last_refresh_status(error_status); EXPECT_THAT(channel->instant_outstanding_rpcs(), - StatusIs(expected_status.code())); + StatusIs(error_status.code())); } TEST(ChannelUsageTest, MakeWeak) {