From be773d7cca06f5555ac1fb201a4892a964bcf112 Mon Sep 17 00:00:00 2001 From: Donna Dionne Date: Tue, 10 Nov 2020 11:14:23 -0800 Subject: [PATCH] Add option to priority policy to ignore reresolution requests from a given child. --- .../lb_policy/priority/priority.cc | 58 ++++++++++++++----- .../client_channel/lb_policy/xds/eds.cc | 1 + 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/core/ext/filters/client_channel/lb_policy/priority/priority.cc b/src/core/ext/filters/client_channel/lb_policy/priority/priority.cc index 95df80b1caf..037eea58372 100644 --- a/src/core/ext/filters/client_channel/lb_policy/priority/priority.cc +++ b/src/core/ext/filters/client_channel/lb_policy/priority/priority.cc @@ -56,23 +56,24 @@ constexpr int kDefaultChildFailoverTimeoutMs = 10000; // Config for priority LB policy. class PriorityLbConfig : public LoadBalancingPolicy::Config { public: - PriorityLbConfig( - std::map> - children, - std::vector priorities) + struct PriorityLbChild { + RefCountedPtr config; + bool ignore_reresolution_requests = false; + }; + + PriorityLbConfig(std::map children, + std::vector priorities) : children_(std::move(children)), priorities_(std::move(priorities)) {} const char* name() const override { return kPriority; } - const std::map>& - children() const { + const std::map& children() const { return children_; } const std::vector& priorities() const { return priorities_; } private: - const std::map> - children_; + const std::map children_; const std::vector priorities_; }; @@ -99,7 +100,8 @@ class PriorityLb : public LoadBalancingPolicy { const std::string& name() const { return name_; } - void UpdateLocked(RefCountedPtr config); + void UpdateLocked(RefCountedPtr config, + bool ignore_reresolution_requests); void ExitIdleLocked(); void ResetBackoffLocked(); void DeactivateLocked(); @@ -184,6 +186,7 @@ class PriorityLb : public LoadBalancingPolicy { RefCountedPtr priority_policy_; const std::string name_; + bool ignore_reresolution_requests_ = false; OrphanablePtr child_policy_; @@ -311,7 +314,8 @@ void PriorityLb::UpdateLocked(UpdateArgs args) { child->DeactivateLocked(); } else { // Existing child found in new config. Update it. - child->UpdateLocked(config_it->second); + child->UpdateLocked(config_it->second.config, + config_it->second.ignore_reresolution_requests); } } // Try to get connected. @@ -425,7 +429,10 @@ void PriorityLb::TryNextPriorityLocked(bool report_connecting) { } child = MakeOrphanable( Ref(DEBUG_LOCATION, "ChildPriority"), child_name); - child->UpdateLocked(config_->children().find(child_name)->second); + auto child_config = config_->children().find(child_name); + GPR_DEBUG_ASSERT(child_config != config_->children().end()); + child->UpdateLocked(child_config->second.config, + child_config->second.ignore_reresolution_requests); return; } // The child already exists. @@ -534,12 +541,14 @@ void PriorityLb::ChildPriority::Orphan() { } void PriorityLb::ChildPriority::UpdateLocked( - RefCountedPtr config) { + RefCountedPtr config, + bool ignore_reresolution_requests) { if (priority_policy_->shutting_down_) return; if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_priority_trace)) { gpr_log(GPR_INFO, "[priority_lb %p] child %s (%p): start update", priority_policy_.get(), name_.c_str(), this); } + ignore_reresolution_requests_ = ignore_reresolution_requests; // Create policy if needed. if (child_policy_ == nullptr) { child_policy_ = CreateChildPolicyLocked(priority_policy_->args_); @@ -731,6 +740,9 @@ void PriorityLb::ChildPriority::OnDeactivationTimerLocked(grpc_error* error) { void PriorityLb::ChildPriority::Helper::RequestReresolution() { if (priority_->priority_policy_->shutting_down_) return; + if (priority_->ignore_reresolution_requests_) { + return; + } priority_->priority_policy_->channel_control_helper()->RequestReresolution(); } @@ -784,7 +796,7 @@ class PriorityLbFactory : public LoadBalancingPolicyFactory { } std::vector error_list; // Children. - std::map> children; + std::map children; auto it = json.object_value().find("children"); if (it == json.object_value().end()) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( @@ -813,6 +825,22 @@ class PriorityLbFactory : public LoadBalancingPolicyFactory { grpc_error* parse_error = GRPC_ERROR_NONE; auto config = LoadBalancingPolicyRegistry::ParseLoadBalancingConfig( it2->second, &parse_error); + bool ignore_resolution_requests = false; + // If present, ignore_reresolution_requests must be of type + // boolean. + auto it3 = + element.object_value().find("ignore_reresolution_requests"); + if (it3 != element.object_value().end()) { + if (it3->second.type() == Json::Type::JSON_TRUE) { + ignore_resolution_requests = true; + } else if (it3->second.type() != Json::Type::JSON_FALSE) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("field:children key:", child_name, + " field:ignore_reresolution_requests:should " + "be type boolean") + .c_str())); + } + } if (config == nullptr) { GPR_DEBUG_ASSERT(parse_error != GRPC_ERROR_NONE); error_list.push_back( @@ -821,7 +849,9 @@ class PriorityLbFactory : public LoadBalancingPolicyFactory { &parse_error, 1)); GRPC_ERROR_UNREF(parse_error); } - children[child_name] = std::move(config); + children[child_name].config = std::move(config); + children[child_name].ignore_reresolution_requests = + ignore_resolution_requests; } } } diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc index 420fb052013..22d7531f53c 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc @@ -622,6 +622,7 @@ EdsLb::CreateChildPolicyConfigLocked() { priority_priorities.emplace_back(child_name); priority_children[child_name] = Json::Object{ {"config", std::move(locality_picking_policy)}, + {"ignore_reresolution_requests", true}, }; } Json json = Json::Array{Json::Object{