diff --git a/src/core/ext/xds/xds_api.cc b/src/core/ext/xds/xds_api.cc index b5395e4408a..a39f51f85b7 100644 --- a/src/core/ext/xds/xds_api.cc +++ b/src/core/ext/xds/xds_api.cc @@ -390,6 +390,19 @@ std::string XdsApi::EdsUpdate::Priority::Locality::ToString() const { absl::StrJoin(endpoint_strings, ", "), "]}"); } +bool XdsApi::EdsUpdate::Priority::operator==(const Priority& other) const { + if (localities.size() != other.localities.size()) return false; + auto it1 = localities.begin(); + auto it2 = other.localities.begin(); + while (it1 != localities.end()) { + if (*it1->first != *it2->first) return false; + if (it1->second != it2->second) return false; + ++it1; + ++it2; + } + return true; +} + std::string XdsApi::EdsUpdate::Priority::ToString() const { std::vector locality_strings; for (const auto& p : localities) { diff --git a/src/core/ext/xds/xds_api.h b/src/core/ext/xds/xds_api.h index d287b901740..f6e0c7db555 100644 --- a/src/core/ext/xds/xds_api.h +++ b/src/core/ext/xds/xds_api.h @@ -194,14 +194,15 @@ class XdsApi { return *name == *other.name && lb_weight == other.lb_weight && endpoints == other.endpoints; } + bool operator!=(const Locality& other) const { + return !(*this == other); + } std::string ToString() const; }; std::map localities; - bool operator==(const Priority& other) const { - return localities == other.localities; - } + bool operator==(const Priority& other) const; std::string ToString() const; }; using PriorityList = absl::InlinedVector; diff --git a/src/core/ext/xds/xds_client.cc b/src/core/ext/xds/xds_client.cc index 23a8c7a519d..d17255fd342 100644 --- a/src/core/ext/xds/xds_client.cc +++ b/src/core/ext/xds/xds_client.cc @@ -1083,21 +1083,14 @@ void XdsClient::ChannelState::AdsCallState::AcceptEdsUpdate( EndpointState& endpoint_state = xds_client()->endpoint_map_[eds_service_name]; // Ignore identical update. - if (endpoint_state.update.has_value()) { - const XdsApi::EdsUpdate& prev_update = endpoint_state.update.value(); - const bool priority_list_changed = - prev_update.priorities != eds_update.priorities; - const bool drop_config_changed = - prev_update.drop_config == nullptr || - *prev_update.drop_config != *eds_update.drop_config; - if (!priority_list_changed && !drop_config_changed) { - if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_trace)) { - gpr_log(GPR_INFO, - "[xds_client %p] EDS update identical to current, ignoring.", - xds_client()); - } - continue; + if (endpoint_state.update.has_value() && + *endpoint_state.update == eds_update) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_trace)) { + gpr_log(GPR_INFO, + "[xds_client %p] EDS update identical to current, ignoring.", + xds_client()); } + continue; } // Update the cluster state. endpoint_state.update = std::move(eds_update); diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc index 57d1430a478..137a3bd7be2 100644 --- a/test/cpp/end2end/xds_end2end_test.cc +++ b/test/cpp/end2end/xds_end2end_test.cc @@ -2083,6 +2083,34 @@ TEST_P(BasicTest, BackendsRestart) { CheckRpcSendOk(1, RpcOptions().set_timeout_ms(2000).set_wait_for_ready(true)); } +TEST_P(BasicTest, IgnoresDuplicateUpdates) { + const size_t kNumRpcsPerAddress = 100; + SetNextResolution({}); + SetNextResolutionForLbChannelAllBalancers(); + AdsServiceImpl::EdsResourceArgs args({ + {"locality0", GetBackendPorts()}, + }); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args)); + // Wait for all backends to come online. + WaitForAllBackends(); + // Send kNumRpcsPerAddress RPCs per server, but send an EDS update in + // between. If the update is not ignored, this will cause the + // round_robin policy to see an update, which will randomly reset its + // position in the address list. + for (size_t i = 0; i < kNumRpcsPerAddress; ++i) { + CheckRpcSendOk(2); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args)); + CheckRpcSendOk(2); + } + // Each backend should have gotten the right number of requests. + for (size_t i = 1; i < backends_.size(); ++i) { + EXPECT_EQ(kNumRpcsPerAddress, + backends_[i]->backend_service()->request_count()); + } +} + using XdsResolverOnlyTest = BasicTest; // Tests switching over from one cluster to another.