[xDS] don't allocate drop config if not needed (#35326)

Closes #35326

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35326 from markdroth:xds_drop_config_cleanup cad30e861e
PiperOrigin-RevId: 591348198
pull/35328/head
Mark D. Roth 12 months ago committed by Copybara-Service
parent 4f7ead2c7b
commit 60deb79b24
  1. 4
      src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc
  2. 19
      src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc
  3. 10
      src/core/ext/xds/xds_endpoint.cc
  4. 6
      src/core/ext/xds/xds_endpoint.h
  5. 21
      test/core/xds/xds_endpoint_resource_type_test.cc

@ -371,7 +371,7 @@ LoadBalancingPolicy::PickResult XdsClusterImplLb::Picker::Pick(
LoadBalancingPolicy::PickArgs args) {
// Handle EDS drops.
const std::string* drop_category;
if (drop_config_->ShouldDrop(&drop_category)) {
if (drop_config_ != nullptr && drop_config_->ShouldDrop(&drop_category)) {
if (drop_stats_ != nullptr) drop_stats_->AddCallDropped(*drop_category);
return PickResult::Drop(absl::UnavailableError(
absl::StrCat("EDS-configured drop: ", *drop_category)));
@ -705,7 +705,7 @@ void XdsClusterImplLbConfig::JsonPostLoad(const Json& json,
// Parse "dropCategories" field.
{
auto value = LoadJsonObjectField<std::vector<DropCategory>>(
json.object(), args, "dropCategories", errors);
json.object(), args, "dropCategories", errors, /*required=*/false);
if (value.has_value()) {
drop_config_ = MakeRefCounted<XdsEndpointResource::DropConfig>();
for (size_t i = 0; i < value->size(); ++i) {

@ -889,8 +889,14 @@ XdsClusterResolverLb::CreateChildPolicyConfigLocked() {
Json::FromObject(std::move(xds_override_host_lb_config))},
})};
// Wrap it in the xds_cluster_impl policy.
Json::Array drop_categories;
Json::Object xds_cluster_impl_config = {
{"clusterName", Json::FromString(discovery_config.cluster_name)},
{"childPolicy", Json::FromArray(std::move(xds_override_host_config))},
{"maxConcurrentRequests",
Json::FromNumber(discovery_config.max_concurrent_requests)},
};
if (discovery_entry.latest_update->drop_config != nullptr) {
Json::Array drop_categories;
for (const auto& category :
discovery_entry.latest_update->drop_config->drop_category_list()) {
drop_categories.push_back(Json::FromObject({
@ -899,14 +905,11 @@ XdsClusterResolverLb::CreateChildPolicyConfigLocked() {
Json::FromNumber(category.parts_per_million)},
}));
}
if (!drop_categories.empty()) {
xds_cluster_impl_config["dropCategories"] =
Json::FromArray(std::move(drop_categories));
}
}
Json::Object xds_cluster_impl_config = {
{"clusterName", Json::FromString(discovery_config.cluster_name)},
{"childPolicy", Json::FromArray(std::move(xds_override_host_config))},
{"dropCategories", Json::FromArray(std::move(drop_categories))},
{"maxConcurrentRequests",
Json::FromNumber(discovery_config.max_concurrent_requests)},
};
if (!discovery_config.eds_service_name.empty()) {
xds_cluster_impl_config["edsServiceName"] =
Json::FromString(discovery_config.eds_service_name);

@ -142,8 +142,9 @@ std::string XdsEndpointResource::ToString() const {
priority_strings.emplace_back(
absl::StrCat("priority ", i, ": ", priority.ToString()));
}
return absl::StrCat("priorities=[", absl::StrJoin(priority_strings, ", "),
"], drop_config=", drop_config->ToString());
return absl::StrCat(
"priorities=[", absl::StrJoin(priority_strings, ", "), "], drop_config=",
drop_config == nullptr ? "<null>" : drop_config->ToString());
}
//
@ -447,7 +448,6 @@ absl::StatusOr<std::shared_ptr<const XdsEndpointResource>> EdsResourceParse(
}
}
// policy
eds_resource->drop_config = MakeRefCounted<XdsEndpointResource::DropConfig>();
const auto* policy = envoy_config_endpoint_v3_ClusterLoadAssignment_policy(
cluster_load_assignment);
if (policy != nullptr) {
@ -456,6 +456,10 @@ absl::StatusOr<std::shared_ptr<const XdsEndpointResource>> EdsResourceParse(
const auto* const* drop_overload =
envoy_config_endpoint_v3_ClusterLoadAssignment_Policy_drop_overloads(
policy, &drop_size);
if (drop_size > 0) {
eds_resource->drop_config =
MakeRefCounted<XdsEndpointResource::DropConfig>();
}
for (size_t i = 0; i < drop_size; ++i) {
ValidationErrors::ScopedField field(
&errors, absl::StrCat(".drop_overloads[", i, "]"));

@ -62,6 +62,7 @@ struct XdsEndpointResource : public XdsResourceType::ResourceData {
std::map<XdsLocalityName*, Locality, XdsLocalityName::Less> localities;
bool operator==(const Priority& other) const;
bool operator!=(const Priority& other) const { return !(*this == other); }
std::string ToString() const;
};
using PriorityList = std::vector<Priority>;
@ -121,7 +122,10 @@ struct XdsEndpointResource : public XdsResourceType::ResourceData {
RefCountedPtr<DropConfig> drop_config;
bool operator==(const XdsEndpointResource& other) const {
return priorities == other.priorities && *drop_config == *other.drop_config;
if (priorities != other.priorities) return false;
if (drop_config == nullptr) return other.drop_config == nullptr;
if (other.drop_config == nullptr) return false;
return *drop_config == *other.drop_config;
}
std::string ToString() const;
};

@ -167,8 +167,7 @@ TEST_F(XdsEndpointTest, MinimumValidConfig) {
.Set(GRPC_ARG_ADDRESS_WEIGHT, 1)
.Set(GRPC_ARG_XDS_HEALTH_STATUS,
XdsHealthStatus::HealthStatus::kUnknown));
ASSERT_NE(resource.drop_config, nullptr);
EXPECT_TRUE(resource.drop_config->drop_category_list().empty());
EXPECT_EQ(resource.drop_config, nullptr);
}
TEST_F(XdsEndpointTest, EndpointWeight) {
@ -214,8 +213,7 @@ TEST_F(XdsEndpointTest, EndpointWeight) {
.Set(GRPC_ARG_ADDRESS_WEIGHT, 3)
.Set(GRPC_ARG_XDS_HEALTH_STATUS,
XdsHealthStatus::HealthStatus::kUnknown));
ASSERT_NE(resource.drop_config, nullptr);
EXPECT_TRUE(resource.drop_config->drop_category_list().empty());
EXPECT_EQ(resource.drop_config, nullptr);
}
TEST_F(XdsEndpointTest, IgnoresLocalityWithNoWeight) {
@ -263,8 +261,7 @@ TEST_F(XdsEndpointTest, IgnoresLocalityWithNoWeight) {
.Set(GRPC_ARG_ADDRESS_WEIGHT, 1)
.Set(GRPC_ARG_XDS_HEALTH_STATUS,
XdsHealthStatus::HealthStatus::kUnknown));
ASSERT_NE(resource.drop_config, nullptr);
EXPECT_TRUE(resource.drop_config->drop_category_list().empty());
EXPECT_EQ(resource.drop_config, nullptr);
}
TEST_F(XdsEndpointTest, IgnoresLocalityWithZeroWeight) {
@ -313,8 +310,7 @@ TEST_F(XdsEndpointTest, IgnoresLocalityWithZeroWeight) {
.Set(GRPC_ARG_ADDRESS_WEIGHT, 1)
.Set(GRPC_ARG_XDS_HEALTH_STATUS,
XdsHealthStatus::HealthStatus::kUnknown));
ASSERT_NE(resource.drop_config, nullptr);
EXPECT_TRUE(resource.drop_config->drop_category_list().empty());
EXPECT_EQ(resource.drop_config, nullptr);
}
TEST_F(XdsEndpointTest, LocalityWithNoEndpoints) {
@ -346,8 +342,7 @@ TEST_F(XdsEndpointTest, LocalityWithNoEndpoints) {
EXPECT_EQ(p.first->sub_zone(), "mysubzone");
EXPECT_EQ(p.second.lb_weight, 1);
EXPECT_EQ(p.second.endpoints.size(), 0);
ASSERT_NE(resource.drop_config, nullptr);
EXPECT_TRUE(resource.drop_config->drop_category_list().empty());
EXPECT_EQ(resource.drop_config, nullptr);
}
TEST_F(XdsEndpointTest, NoLocality) {
@ -544,8 +539,7 @@ TEST_F(XdsEndpointTest, MultipleAddressesPerEndpoint) {
.Set(GRPC_ARG_ADDRESS_WEIGHT, 1)
.Set(GRPC_ARG_XDS_HEALTH_STATUS,
XdsHealthStatus::HealthStatus::kUnknown));
ASSERT_NE(resource.drop_config, nullptr);
EXPECT_TRUE(resource.drop_config->drop_category_list().empty());
EXPECT_EQ(resource.drop_config, nullptr);
}
TEST_F(XdsEndpointTest, AdditionalAddressesMissingAddress) {
@ -735,8 +729,7 @@ TEST_F(XdsEndpointTest, IgnoresMultipleAddressesPerEndpointWhenNotEnabled) {
.Set(GRPC_ARG_ADDRESS_WEIGHT, 1)
.Set(GRPC_ARG_XDS_HEALTH_STATUS,
XdsHealthStatus::HealthStatus::kUnknown));
ASSERT_NE(resource.drop_config, nullptr);
EXPECT_TRUE(resource.drop_config->drop_category_list().empty());
EXPECT_EQ(resource.drop_config, nullptr);
}
TEST_F(XdsEndpointTest, MissingEndpoint) {

Loading…
Cancel
Save