[xDS] remove unnecessary string from XdsConfig struct (#35503)

I realized that this field wasn't actually necessary, since the string is already present in the map key.

Closes #35503

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35503 from markdroth:xds_config_remove_cluster_name 94d5edc133
PiperOrigin-RevId: 597375018
pull/34966/head
Mark D. Roth 11 months ago committed by Copybara-Service
parent 2350de3936
commit a446df61f4
  1. 14
      src/core/ext/filters/client_channel/lb_policy/xds/cds.cc
  2. 22
      src/core/ext/filters/client_channel/resolver/xds/xds_dependency_manager.cc
  3. 24
      src/core/ext/filters/client_channel/resolver/xds/xds_dependency_manager.h
  4. 4
      test/core/client_channel/lb_policy/xds_override_host_test.cc

@ -404,7 +404,7 @@ absl::Status CdsLb::UpdateLocked(UpdateArgs args) {
old_cluster_config, *new_cluster_config, endpoint_config);
// Populate addresses and resolution_note for child policy.
update_args.addresses = std::make_shared<PriorityEndpointIterator>(
new_cluster_config->cluster_name, endpoint_config.endpoints,
cluster_name_, endpoint_config.endpoints,
child_name_state_.priority_child_numbers);
update_args.resolution_note = endpoint_config.resolution_note;
// Construct child policy config.
@ -476,8 +476,7 @@ CdsLb::ChildNameState CdsLb::ComputeChildNames(
locality_child_map;
std::map<size_t, std::set<XdsLocalityName*, XdsLocalityName::Less>>
child_locality_map;
if (old_cluster != nullptr &&
old_cluster->cluster_name == new_cluster.cluster_name) {
if (old_cluster != nullptr) {
auto* old_endpoint_config =
absl::get_if<XdsConfig::ClusterConfig::EndpointConfig>(
&old_cluster->children);
@ -579,9 +578,8 @@ Json CdsLb::CreateChildPolicyConfigForLeafCluster(
GetUpdatePriorityList(endpoint_config.endpoints.get());
for (size_t priority = 0; priority < priority_list.size(); ++priority) {
// Add priority entry, with the appropriate child name.
std::string child_name =
MakeChildPolicyName(new_cluster.cluster_name,
child_name_state_.priority_child_numbers[priority]);
std::string child_name = MakeChildPolicyName(
cluster_name_, child_name_state_.priority_child_numbers[priority]);
priority_priorities.emplace_back(Json::FromString(child_name));
Json::Object child_config = {{"config", xds_lb_policy}};
if (!is_logical_dns) {
@ -600,7 +598,7 @@ Json CdsLb::CreateChildPolicyConfigForLeafCluster(
Json xds_override_host_policy = Json::FromArray({Json::FromObject({
{"xds_override_host_experimental",
Json::FromObject({
{"clusterName", Json::FromString(new_cluster.cluster_name)},
{"clusterName", Json::FromString(cluster_name_)},
{"childPolicy", std::move(priority_policy)},
})},
})});
@ -608,7 +606,7 @@ Json CdsLb::CreateChildPolicyConfigForLeafCluster(
Json xds_cluster_impl_policy = Json::FromArray({Json::FromObject({
{"xds_cluster_impl_experimental",
Json::FromObject({
{"clusterName", Json::FromString(new_cluster.cluster_name)},
{"clusterName", Json::FromString(cluster_name_)},
{"childPolicy", std::move(xds_override_host_policy)},
})},
})});

@ -41,19 +41,17 @@ constexpr int kMaxXdsAggregateClusterRecursionDepth = 16;
//
XdsDependencyManager::XdsConfig::ClusterConfig::ClusterConfig(
std::string cluster_name, std::shared_ptr<const XdsClusterResource> cluster,
std::shared_ptr<const XdsClusterResource> cluster,
std::shared_ptr<const XdsEndpointResource> endpoints,
std::string resolution_note)
: cluster_name(std::move(cluster_name)),
cluster(std::move(cluster)),
: cluster(std::move(cluster)),
children(absl::in_place_type_t<EndpointConfig>(), std::move(endpoints),
std::move(resolution_note)) {}
XdsDependencyManager::XdsConfig::ClusterConfig::ClusterConfig(
std::string cluster_name, std::shared_ptr<const XdsClusterResource> cluster,
std::shared_ptr<const XdsClusterResource> cluster,
std::vector<absl::string_view> leaf_clusters)
: cluster_name(std::move(cluster_name)),
cluster(std::move(cluster)),
: cluster(std::move(cluster)),
children(absl::in_place_type_t<AggregateConfig>(),
std::move(leaf_clusters)) {}
@ -75,9 +73,6 @@ std::string XdsDependencyManager::XdsConfig::ToString() const {
} else {
parts.push_back(
absl::StrCat(" {\n"
" name: \"",
p.second->cluster_name,
"\"\n"
" cluster: {",
p.second->cluster->ToString(), "}\n"));
Match(
@ -794,8 +789,7 @@ bool XdsDependencyManager::PopulateClusterConfigMap(
return false;
}
// Populate cluster config.
cluster_config.emplace(std::string(name), *state.update,
eds_state.update.endpoints,
cluster_config.emplace(*state.update, eds_state.update.endpoints,
eds_state.update.resolution_note);
if (leaf_clusters != nullptr) (*leaf_clusters)->push_back(name);
return true;
@ -845,8 +839,7 @@ bool XdsDependencyManager::PopulateClusterConfigMap(
return false;
}
// Populate cluster config.
cluster_config.emplace(std::string(name), *state.update,
dns_state.update.endpoints,
cluster_config.emplace(*state.update, dns_state.update.endpoints,
dns_state.update.resolution_note);
if (leaf_clusters != nullptr) (*leaf_clusters)->push_back(name);
return true;
@ -901,8 +894,7 @@ bool XdsDependencyManager::PopulateClusterConfigMap(
// at the root of the tree, because we need to make sure the list
// of underlying cluster names stays alive so that the leaf cluster
// list of the root aggregate cluster can point to those strings.
aggregate_cluster_config.emplace(std::string(name),
std::move(cluster_resource),
aggregate_cluster_config.emplace(std::move(cluster_resource),
std::move(*child_leaf_clusters));
return have_all_resources;
});

@ -39,7 +39,7 @@ class XdsDependencyManager : public RefCounted<XdsDependencyManager>,
public Orphanable {
public:
struct XdsConfig : public RefCounted<XdsConfig> {
// Listener resource.
// Listener resource. Always non-null.
std::shared_ptr<const XdsListenerResource> listener;
// RouteConfig resource. Will be populated even if RouteConfig is
// inlined into the Listener resource.
@ -47,15 +47,11 @@ class XdsDependencyManager : public RefCounted<XdsDependencyManager>,
// Virtual host. Points into route_config. Will always be non-null.
const XdsRouteConfigResource::VirtualHost* virtual_host;
// Cluster map. A cluster will have a non-OK status if either
// (a) there was an error and we did not already have a valid
// resource or (b) the resource does not exist.
struct ClusterConfig {
// Cluster name and resource.
std::string cluster_name;
// Cluster resource. Always non-null.
std::shared_ptr<const XdsClusterResource> cluster;
// Endpoint info. If there was an error, endpoints will be null
// and resolution_note will be set. Not used for aggregate clusters.
// Endpoint info for EDS and LOGICAL_DNS clusters. If there was an
// error, endpoints will be null and resolution_note will be set.
struct EndpointConfig {
std::shared_ptr<const XdsEndpointResource> endpoints;
std::string resolution_note;
@ -82,20 +78,20 @@ class XdsDependencyManager : public RefCounted<XdsDependencyManager>,
absl::variant<EndpointConfig, AggregateConfig> children;
// Ctor for leaf clusters.
ClusterConfig(std::string cluster_name,
std::shared_ptr<const XdsClusterResource> cluster,
ClusterConfig(std::shared_ptr<const XdsClusterResource> cluster,
std::shared_ptr<const XdsEndpointResource> endpoints,
std::string resolution_note);
// Ctor for aggregate clusters.
ClusterConfig(std::string cluster_name,
std::shared_ptr<const XdsClusterResource> cluster,
ClusterConfig(std::shared_ptr<const XdsClusterResource> cluster,
std::vector<absl::string_view> leaf_clusters);
bool operator==(const ClusterConfig& other) const {
return cluster_name == other.cluster_name && cluster == other.cluster &&
children == other.children;
return cluster == other.cluster && children == other.children;
}
};
// Cluster map. A cluster will have a non-OK status if either
// (a) there was an error and we did not already have a valid
// resource or (b) the resource does not exist.
absl::flat_hash_map<std::string, absl::StatusOr<ClusterConfig>> clusters;
std::string ToString() const;

@ -72,8 +72,8 @@ class XdsOverrideHostTest : public LoadBalancingPolicyTest {
cluster_resource->connection_idle_timeout = *connection_idle_timeout;
}
auto xds_config = MakeRefCounted<XdsDependencyManager::XdsConfig>();
xds_config->clusters[cluster_name].emplace(
cluster_name, std::move(cluster_resource), nullptr, "");
xds_config->clusters[std::move(cluster_name)].emplace(
std::move(cluster_resource), nullptr, "");
return xds_config;
}

Loading…
Cancel
Save