From 2afe013e8489fb9327b589180351faa7a664550f Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 13 Sep 2024 12:59:42 -0700 Subject: [PATCH] [flake] Fix chaotic good no_logging flake (#37721) Move the problematic log to be a trace log (not needed for general workflows), and take the opportunity to clean up a few other errors to log every n seconds -- because we principally need the signal that it's happening, not every occurrence. Closes #37721 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/37721 from ctiller:flake-fightas-6 ac18237c51ec8b635c0ae15f246953248df9a8ff PiperOrigin-RevId: 674404222 --- .../server/chaotic_good_server.cc | 22 ++++++++----------- .../chaotic_good/server/chaotic_good_server.h | 2 +- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/core/ext/transport/chaotic_good/server/chaotic_good_server.cc b/src/core/ext/transport/chaotic_good/server/chaotic_good_server.cc index 2a707602920..ac044244bd2 100644 --- a/src/core/ext/transport/chaotic_good/server/chaotic_good_server.cc +++ b/src/core/ext/transport/chaotic_good/server/chaotic_good_server.cc @@ -187,11 +187,7 @@ void ChaoticGoodServerListener::ActiveConnection::NewConnectionID() { connection_id_, std::make_shared>()); } -void ChaoticGoodServerListener::ActiveConnection::Done( - absl::optional error) { - if (error.has_value()) { - LOG(ERROR) << "ActiveConnection::Done:" << this << " " << *error; - } +void ChaoticGoodServerListener::ActiveConnection::Done() { // Can easily be holding various locks here: bounce through EE to ensure no // deadlocks. listener_->event_engine_->Run([self = Ref()]() { @@ -387,13 +383,15 @@ auto ChaoticGoodServerListener::ActiveConnection::HandshakingState:: void ChaoticGoodServerListener::ActiveConnection::HandshakingState:: OnHandshakeDone(absl::StatusOr result) { if (!result.ok()) { - connection_->Done( - absl::StrCat("Handshake failed: ", result.status().ToString())); + LOG_EVERY_N_SEC(ERROR, 5) << "Handshake failed: ", result.status(); + connection_->Done(); return; } CHECK_NE(*result, nullptr); if ((*result)->endpoint == nullptr) { - connection_->Done("Server handshake done but has empty endpoint."); + LOG_EVERY_N_SEC(ERROR, 5) + << "Server handshake done but has empty endpoint."; + connection_->Done(); return; } CHECK(grpc_event_engine::experimental::grpc_is_event_engine_endpoint( @@ -429,12 +427,10 @@ void ChaoticGoodServerListener::ActiveConnection::HandshakingState:: EventEngineWakeupScheduler(connection_->listener_->event_engine_), [self = Ref()](absl::Status status) { if (!status.ok()) { - self->connection_->Done( - absl::StrCat("Server setting frame handling failed: ", - StatusToString(status))); - } else { - self->connection_->Done(); + GRPC_TRACE_LOG(chaotic_good, ERROR) + << "Server setting frame handling failed: " << status; } + self->connection_->Done(); }, connection_->arena_.get()); MutexLock lock(&connection_->mu_); diff --git a/src/core/ext/transport/chaotic_good/server/chaotic_good_server.h b/src/core/ext/transport/chaotic_good/server/chaotic_good_server.h index 0b20dcbb363..d5ec23b5de3 100644 --- a/src/core/ext/transport/chaotic_good/server/chaotic_good_server.h +++ b/src/core/ext/transport/chaotic_good/server/chaotic_good_server.h @@ -111,7 +111,7 @@ class ChaoticGoodServerListener final : public Server::ListenerInterface { }; private: - void Done(absl::optional error = absl::nullopt); + void Done(); void NewConnectionID(); RefCountedPtr arena_ = SimpleArenaAllocator()->MakeArena(); const RefCountedPtr listener_;