Merge pull request #22537 from gnossen/cherrypick_22525
Backport #22525 to 1.28 (Report RPC failures to LB recv_trailing_metadata callbacks.)
diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc
index 977384d..eba4059 100644
--- a/src/core/ext/filters/client_channel/client_channel.cc
+++ b/src/core/ext/filters/client_channel/client_channel.cc
@@ -2283,10 +2283,32 @@
void CallData::RecvTrailingMetadataReadyForLoadBalancingPolicy(
void* arg, grpc_error* error) {
CallData* calld = static_cast<CallData*>(arg);
+ // Set error if call did not succeed.
+ grpc_error* error_for_lb = GRPC_ERROR_NONE;
+ if (error != GRPC_ERROR_NONE) {
+ error_for_lb = error;
+ } else {
+ const auto& fields = calld->recv_trailing_metadata_->idx.named;
+ GPR_ASSERT(fields.grpc_status != nullptr);
+ grpc_status_code status =
+ grpc_get_status_code_from_metadata(fields.grpc_status->md);
+ std::string msg;
+ if (status != GRPC_STATUS_OK) {
+ error_for_lb = grpc_error_set_int(
+ GRPC_ERROR_CREATE_FROM_STATIC_STRING("call failed"),
+ GRPC_ERROR_INT_GRPC_STATUS, status);
+ if (fields.grpc_message != nullptr) {
+ error_for_lb = grpc_error_set_str(
+ error_for_lb, GRPC_ERROR_STR_GRPC_MESSAGE,
+ grpc_slice_ref_internal(GRPC_MDVALUE(fields.grpc_message->md)));
+ }
+ }
+ }
// Invoke callback to LB policy.
Metadata trailing_metadata(calld, calld->recv_trailing_metadata_);
- calld->lb_recv_trailing_metadata_ready_(error, &trailing_metadata,
+ calld->lb_recv_trailing_metadata_ready_(error_for_lb, &trailing_metadata,
&calld->lb_call_state_);
+ if (error == GRPC_ERROR_NONE) GRPC_ERROR_UNREF(error_for_lb);
// Chain to original callback.
Closure::Run(DEBUG_LOCATION, calld->original_recv_trailing_metadata_ready_,
GRPC_ERROR_REF(error));
diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc
index 10261f3..9719086 100644
--- a/test/cpp/end2end/xds_end2end_test.cc
+++ b/test/cpp/end2end/xds_end2end_test.cc
@@ -1343,11 +1343,15 @@
}
Status SendRpc(EchoResponse* response = nullptr, int timeout_ms = 1000,
- bool wait_for_ready = false) {
+ bool wait_for_ready = false, bool server_fail = false) {
const bool local_response = (response == nullptr);
if (local_response) response = new EchoResponse;
EchoRequest request;
request.set_message(kRequestMessage_);
+ if (server_fail) {
+ request.mutable_param()->mutable_expected_error()->set_code(
+ GRPC_STATUS_FAILED_PRECONDITION);
+ }
ClientContext context;
context.set_deadline(grpc_timeout_milliseconds_to_deadline(timeout_ms));
if (wait_for_ready) context.set_wait_for_ready(true);
@@ -1367,9 +1371,11 @@
}
}
- void CheckRpcSendFailure() {
- const Status status = SendRpc();
- EXPECT_FALSE(status.ok());
+ void CheckRpcSendFailure(const size_t times = 1, bool server_fail = false) {
+ for (size_t i = 0; i < times; ++i) {
+ const Status status = SendRpc(nullptr, 1000, false, server_fail);
+ EXPECT_FALSE(status.ok());
+ }
}
public:
@@ -3437,7 +3443,8 @@
TEST_P(ClientLoadReportingTest, Vanilla) {
SetNextResolution({});
SetNextResolutionForLbChannel({balancers_[0]->port()});
- const size_t kNumRpcsPerAddress = 100;
+ const size_t kNumRpcsPerAddress = 10;
+ const size_t kNumFailuresPerAddress = 3;
// TODO(juanlishen): Partition the backends after multiple localities is
// tested.
AdsServiceImpl::EdsResourceArgs args({
@@ -3452,9 +3459,11 @@
std::tie(num_ok, num_failure, num_drops) = WaitForAllBackends();
// Send kNumRpcsPerAddress RPCs per server.
CheckRpcSendOk(kNumRpcsPerAddress * num_backends_);
- // Each backend should have gotten 100 requests.
+ CheckRpcSendFailure(kNumFailuresPerAddress * num_backends_,
+ /*server_fail=*/true);
+ // Check that each backend got the right number of requests.
for (size_t i = 0; i < backends_.size(); ++i) {
- EXPECT_EQ(kNumRpcsPerAddress,
+ EXPECT_EQ(kNumRpcsPerAddress + kNumFailuresPerAddress,
backends_[i]->backend_service()->request_count());
}
// The LRS service got a single request, and sent a single response.
@@ -3468,9 +3477,11 @@
EXPECT_EQ(kNumRpcsPerAddress * num_backends_ + num_ok,
client_stats.total_successful_requests());
EXPECT_EQ(0U, client_stats.total_requests_in_progress());
- EXPECT_EQ(kNumRpcsPerAddress * num_backends_ + num_ok,
+ EXPECT_EQ((kNumRpcsPerAddress + kNumFailuresPerAddress) * num_backends_ +
+ num_ok + num_failure,
client_stats.total_issued_requests());
- EXPECT_EQ(0U, client_stats.total_error_requests());
+ EXPECT_EQ(kNumFailuresPerAddress * num_backends_ + num_failure,
+ client_stats.total_error_requests());
EXPECT_EQ(0U, client_stats.total_dropped_requests());
}
