From 856414fb52dfcf233180624d42748fb398974567 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 12 Sep 2023 20:04:33 +0000 Subject: [PATCH] make experiment disablable --- bazel/experiments.bzl | 18 + src/core/BUILD | 3 + .../lb_policy/round_robin/round_robin.cc | 452 +++++++++++++++++- src/core/lib/experiments/experiments.cc | 30 +- src/core/lib/experiments/experiments.h | 18 +- src/core/lib/experiments/experiments.yaml | 4 +- src/core/lib/experiments/rollouts.yaml | 2 +- test/cpp/end2end/BUILD | 2 + 8 files changed, 505 insertions(+), 24 deletions(-) diff --git a/bazel/experiments.bzl b/bazel/experiments.bzl index 4285419f54d27..9ed9bd2c48111 100644 --- a/bazel/experiments.bzl +++ b/bazel/experiments.bzl @@ -60,9 +60,15 @@ EXPERIMENTS = { "core_end2end_test": [ "work_stealing", ], + "cpp_lb_end2end_test": [ + "round_robin_delegate_to_pick_first", + ], "flow_control_test": [ "lazier_stream_updates", ], + "xds_end2end_test": [ + "round_robin_delegate_to_pick_first", + ], }, }, "ios": { @@ -108,9 +114,15 @@ EXPERIMENTS = { "core_end2end_test": [ "work_stealing", ], + "cpp_lb_end2end_test": [ + "round_robin_delegate_to_pick_first", + ], "flow_control_test": [ "lazier_stream_updates", ], + "xds_end2end_test": [ + "round_robin_delegate_to_pick_first", + ], }, }, "posix": { @@ -166,9 +178,15 @@ EXPERIMENTS = { "core_end2end_test": [ "work_stealing", ], + "cpp_lb_end2end_test": [ + "round_robin_delegate_to_pick_first", + ], "flow_control_test": [ "lazier_stream_updates", ], + "xds_end2end_test": [ + "round_robin_delegate_to_pick_first", + ], }, }, } diff --git a/src/core/BUILD b/src/core/BUILD index 3577f63855c09..a8474c2d84e58 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -4914,10 +4914,13 @@ grpc_cc_library( language = "c++", deps = [ "channel_args", + "experiments", + "grpc_lb_subchannel_list", "json", "lb_endpoint_list", "lb_policy", "lb_policy_factory", + "subchannel_interface", "//:config", "//:debug_location", "//:gpr", diff --git a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc index d883fe0c7ccbe..2a556d6448903 100644 --- a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc +++ b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc @@ -38,9 +38,11 @@ #include #include "src/core/ext/filters/client_channel/lb_policy/endpoint_list.h" +#include "src/core/ext/filters/client_channel/lb_policy/subchannel_list.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/debug/trace.h" +#include "src/core/lib/experiments/experiments.h" #include "src/core/lib/gprpp/debug_location.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" @@ -48,6 +50,7 @@ #include "src/core/lib/json/json.h" #include "src/core/lib/load_balancing/lb_policy.h" #include "src/core/lib/load_balancing/lb_policy_factory.h" +#include "src/core/lib/load_balancing/subchannel_interface.h" #include "src/core/lib/resolver/server_address.h" #include "src/core/lib/transport/connectivity_state.h" @@ -58,11 +61,455 @@ TraceFlag grpc_lb_round_robin_trace(false, "round_robin"); namespace { // -// round_robin LB policy +// legacy round_robin LB policy (before dualstack support) // constexpr absl::string_view kRoundRobin = "round_robin"; +class OldRoundRobin : public LoadBalancingPolicy { + public: + explicit OldRoundRobin(Args args); + + absl::string_view name() const override { return kRoundRobin; } + + absl::Status UpdateLocked(UpdateArgs args) override; + void ResetBackoffLocked() override; + + private: + ~OldRoundRobin() override; + + // Forward declaration. + class RoundRobinSubchannelList; + + // Data for a particular subchannel in a subchannel list. + // This subclass adds the following functionality: + // - Tracks the previous connectivity state of the subchannel, so that + // we know how many subchannels are in each state. + class RoundRobinSubchannelData + : public SubchannelData { + public: + RoundRobinSubchannelData( + SubchannelList* + subchannel_list, + const ServerAddress& address, + RefCountedPtr subchannel) + : SubchannelData(subchannel_list, address, std::move(subchannel)) {} + + absl::optional connectivity_state() const { + return logical_connectivity_state_; + } + + private: + // Performs connectivity state updates that need to be done only + // after we have started watching. + void ProcessConnectivityChangeLocked( + absl::optional old_state, + grpc_connectivity_state new_state) override; + + // Updates the logical connectivity state. + void UpdateLogicalConnectivityStateLocked( + grpc_connectivity_state connectivity_state); + + // The logical connectivity state of the subchannel. + // Note that the logical connectivity state may differ from the + // actual reported state in some cases (e.g., after we see + // TRANSIENT_FAILURE, we ignore any subsequent state changes until + // we see READY). + absl::optional logical_connectivity_state_; + }; + + // A list of subchannels. + class RoundRobinSubchannelList + : public SubchannelList { + public: + RoundRobinSubchannelList(OldRoundRobin* policy, ServerAddressList addresses, + const ChannelArgs& args) + : SubchannelList(policy, + (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace) + ? "RoundRobinSubchannelList" + : nullptr), + std::move(addresses), policy->channel_control_helper(), + args) { + // Need to maintain a ref to the LB policy as long as we maintain + // any references to subchannels, since the subchannels' + // pollset_sets will include the LB policy's pollset_set. + policy->Ref(DEBUG_LOCATION, "subchannel_list").release(); + } + + ~RoundRobinSubchannelList() override { + OldRoundRobin* p = static_cast(policy()); + p->Unref(DEBUG_LOCATION, "subchannel_list"); + } + + // Updates the counters of subchannels in each state when a + // subchannel transitions from old_state to new_state. + void UpdateStateCountersLocked( + absl::optional old_state, + grpc_connectivity_state new_state); + + // Ensures that the right subchannel list is used and then updates + // the RR policy's connectivity state based on the subchannel list's + // state counters. + void MaybeUpdateRoundRobinConnectivityStateLocked( + absl::Status status_for_tf); + + private: + std::shared_ptr work_serializer() const override { + return static_cast(policy())->work_serializer(); + } + + std::string CountersString() const { + return absl::StrCat("num_subchannels=", num_subchannels(), + " num_ready=", num_ready_, + " num_connecting=", num_connecting_, + " num_transient_failure=", num_transient_failure_); + } + + size_t num_ready_ = 0; + size_t num_connecting_ = 0; + size_t num_transient_failure_ = 0; + + absl::Status last_failure_; + }; + + class Picker : public SubchannelPicker { + public: + Picker(OldRoundRobin* parent, RoundRobinSubchannelList* subchannel_list); + + PickResult Pick(PickArgs args) override; + + private: + // Using pointer value only, no ref held -- do not dereference! + OldRoundRobin* parent_; + + std::atomic last_picked_index_; + std::vector> subchannels_; + }; + + void ShutdownLocked() override; + + // List of subchannels. + RefCountedPtr subchannel_list_; + // Latest pending subchannel list. + // When we get an updated address list, we create a new subchannel list + // for it here, and we wait to swap it into subchannel_list_ until the new + // list becomes READY. + RefCountedPtr latest_pending_subchannel_list_; + + bool shutdown_ = false; + + absl::BitGen bit_gen_; +}; + +// +// OldRoundRobin::Picker +// + +OldRoundRobin::Picker::Picker(OldRoundRobin* parent, + RoundRobinSubchannelList* subchannel_list) + : parent_(parent) { + for (size_t i = 0; i < subchannel_list->num_subchannels(); ++i) { + RoundRobinSubchannelData* sd = subchannel_list->subchannel(i); + if (sd->connectivity_state().value_or(GRPC_CHANNEL_IDLE) == + GRPC_CHANNEL_READY) { + subchannels_.push_back(sd->subchannel()->Ref()); + } + } + // For discussion on why we generate a random starting index for + // the picker, see https://github.com/grpc/grpc-go/issues/2580. + size_t index = + absl::Uniform(parent->bit_gen_, 0, subchannels_.size()); + last_picked_index_.store(index, std::memory_order_relaxed); + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, + "[RR %p picker %p] created picker from subchannel_list=%p " + "with %" PRIuPTR " READY subchannels; last_picked_index_=%" PRIuPTR, + parent_, this, subchannel_list, subchannels_.size(), index); + } +} + +OldRoundRobin::PickResult OldRoundRobin::Picker::Pick(PickArgs /*args*/) { + size_t index = last_picked_index_.fetch_add(1, std::memory_order_relaxed) % + subchannels_.size(); + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, + "[RR %p picker %p] returning index %" PRIuPTR ", subchannel=%p", + parent_, this, index, subchannels_[index].get()); + } + return PickResult::Complete(subchannels_[index]); +} + +// +// RoundRobin +// + +OldRoundRobin::OldRoundRobin(Args args) : LoadBalancingPolicy(std::move(args)) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, "[RR %p] Created", this); + } +} + +OldRoundRobin::~OldRoundRobin() { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, "[RR %p] Destroying Round Robin policy", this); + } + GPR_ASSERT(subchannel_list_ == nullptr); + GPR_ASSERT(latest_pending_subchannel_list_ == nullptr); +} + +void OldRoundRobin::ShutdownLocked() { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, "[RR %p] Shutting down", this); + } + shutdown_ = true; + subchannel_list_.reset(); + latest_pending_subchannel_list_.reset(); +} + +void OldRoundRobin::ResetBackoffLocked() { + subchannel_list_->ResetBackoffLocked(); + if (latest_pending_subchannel_list_ != nullptr) { + latest_pending_subchannel_list_->ResetBackoffLocked(); + } +} + +absl::Status OldRoundRobin::UpdateLocked(UpdateArgs args) { + ServerAddressList addresses; + if (args.addresses.ok()) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, "[RR %p] received update with %" PRIuPTR " addresses", + this, args.addresses->size()); + } + addresses = std::move(*args.addresses); + } else { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, "[RR %p] received update with address error: %s", this, + args.addresses.status().ToString().c_str()); + } + // If we already have a subchannel list, then keep using the existing + // list, but still report back that the update was not accepted. + if (subchannel_list_ != nullptr) return args.addresses.status(); + } + // Create new subchannel list, replacing the previous pending list, if any. + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace) && + latest_pending_subchannel_list_ != nullptr) { + gpr_log(GPR_INFO, "[RR %p] replacing previous pending subchannel list %p", + this, latest_pending_subchannel_list_.get()); + } + latest_pending_subchannel_list_ = MakeRefCounted( + this, std::move(addresses), args.args); + latest_pending_subchannel_list_->StartWatchingLocked(args.args); + // If the new list is empty, immediately promote it to + // subchannel_list_ and report TRANSIENT_FAILURE. + if (latest_pending_subchannel_list_->num_subchannels() == 0) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace) && + subchannel_list_ != nullptr) { + gpr_log(GPR_INFO, "[RR %p] replacing previous subchannel list %p", this, + subchannel_list_.get()); + } + subchannel_list_ = std::move(latest_pending_subchannel_list_); + absl::Status status = + args.addresses.ok() ? absl::UnavailableError(absl::StrCat( + "empty address list: ", args.resolution_note)) + : args.addresses.status(); + channel_control_helper()->UpdateState( + GRPC_CHANNEL_TRANSIENT_FAILURE, status, + MakeRefCounted(status)); + return status; + } + // Otherwise, if this is the initial update, immediately promote it to + // subchannel_list_. + if (subchannel_list_.get() == nullptr) { + subchannel_list_ = std::move(latest_pending_subchannel_list_); + } + return absl::OkStatus(); +} + +// +// RoundRobinSubchannelList +// + +void OldRoundRobin::RoundRobinSubchannelList::UpdateStateCountersLocked( + absl::optional old_state, + grpc_connectivity_state new_state) { + if (old_state.has_value()) { + GPR_ASSERT(*old_state != GRPC_CHANNEL_SHUTDOWN); + if (*old_state == GRPC_CHANNEL_READY) { + GPR_ASSERT(num_ready_ > 0); + --num_ready_; + } else if (*old_state == GRPC_CHANNEL_CONNECTING) { + GPR_ASSERT(num_connecting_ > 0); + --num_connecting_; + } else if (*old_state == GRPC_CHANNEL_TRANSIENT_FAILURE) { + GPR_ASSERT(num_transient_failure_ > 0); + --num_transient_failure_; + } + } + GPR_ASSERT(new_state != GRPC_CHANNEL_SHUTDOWN); + if (new_state == GRPC_CHANNEL_READY) { + ++num_ready_; + } else if (new_state == GRPC_CHANNEL_CONNECTING) { + ++num_connecting_; + } else if (new_state == GRPC_CHANNEL_TRANSIENT_FAILURE) { + ++num_transient_failure_; + } +} + +void OldRoundRobin::RoundRobinSubchannelList:: + MaybeUpdateRoundRobinConnectivityStateLocked(absl::Status status_for_tf) { + OldRoundRobin* p = static_cast(policy()); + // If this is latest_pending_subchannel_list_, then swap it into + // subchannel_list_ in the following cases: + // - subchannel_list_ has no READY subchannels. + // - This list has at least one READY subchannel and we have seen the + // initial connectivity state notification for all subchannels. + // - All of the subchannels in this list are in TRANSIENT_FAILURE. + // (This may cause the channel to go from READY to TRANSIENT_FAILURE, + // but we're doing what the control plane told us to do.) + if (p->latest_pending_subchannel_list_.get() == this && + (p->subchannel_list_->num_ready_ == 0 || + (num_ready_ > 0 && AllSubchannelsSeenInitialState()) || + num_transient_failure_ == num_subchannels())) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + const std::string old_counters_string = + p->subchannel_list_ != nullptr ? p->subchannel_list_->CountersString() + : ""; + gpr_log( + GPR_INFO, + "[RR %p] swapping out subchannel list %p (%s) in favor of %p (%s)", p, + p->subchannel_list_.get(), old_counters_string.c_str(), this, + CountersString().c_str()); + } + p->subchannel_list_ = std::move(p->latest_pending_subchannel_list_); + } + // Only set connectivity state if this is the current subchannel list. + if (p->subchannel_list_.get() != this) return; + // First matching rule wins: + // 1) ANY subchannel is READY => policy is READY. + // 2) ANY subchannel is CONNECTING => policy is CONNECTING. + // 3) ALL subchannels are TRANSIENT_FAILURE => policy is TRANSIENT_FAILURE. + if (num_ready_ > 0) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, "[RR %p] reporting READY with subchannel list %p", p, + this); + } + p->channel_control_helper()->UpdateState(GRPC_CHANNEL_READY, absl::Status(), + MakeRefCounted(p, this)); + } else if (num_connecting_ > 0) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, "[RR %p] reporting CONNECTING with subchannel list %p", + p, this); + } + p->channel_control_helper()->UpdateState( + GRPC_CHANNEL_CONNECTING, absl::Status(), + MakeRefCounted(p->Ref(DEBUG_LOCATION, "QueuePicker"))); + } else if (num_transient_failure_ == num_subchannels()) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, + "[RR %p] reporting TRANSIENT_FAILURE with subchannel list %p: %s", + p, this, status_for_tf.ToString().c_str()); + } + if (!status_for_tf.ok()) { + last_failure_ = absl::UnavailableError( + absl::StrCat("connections to all backends failing; last error: ", + status_for_tf.ToString())); + } + p->channel_control_helper()->UpdateState( + GRPC_CHANNEL_TRANSIENT_FAILURE, last_failure_, + MakeRefCounted(last_failure_)); + } +} + +// +// RoundRobinSubchannelData +// + +void OldRoundRobin::RoundRobinSubchannelData::ProcessConnectivityChangeLocked( + absl::optional old_state, + grpc_connectivity_state new_state) { + OldRoundRobin* p = static_cast(subchannel_list()->policy()); + GPR_ASSERT(subchannel() != nullptr); + // If this is not the initial state notification and the new state is + // TRANSIENT_FAILURE or IDLE, re-resolve. + // Note that we don't want to do this on the initial state notification, + // because that would result in an endless loop of re-resolution. + if (old_state.has_value() && (new_state == GRPC_CHANNEL_TRANSIENT_FAILURE || + new_state == GRPC_CHANNEL_IDLE)) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, + "[RR %p] Subchannel %p reported %s; requesting re-resolution", p, + subchannel(), ConnectivityStateName(new_state)); + } + p->channel_control_helper()->RequestReresolution(); + } + if (new_state == GRPC_CHANNEL_IDLE) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, + "[RR %p] Subchannel %p reported IDLE; requesting connection", p, + subchannel()); + } + subchannel()->RequestConnection(); + } + // Update logical connectivity state. + UpdateLogicalConnectivityStateLocked(new_state); + // Update the policy state. + subchannel_list()->MaybeUpdateRoundRobinConnectivityStateLocked( + connectivity_status()); +} + +void OldRoundRobin::RoundRobinSubchannelData:: + UpdateLogicalConnectivityStateLocked( + grpc_connectivity_state connectivity_state) { + OldRoundRobin* p = static_cast(subchannel_list()->policy()); + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log( + GPR_INFO, + "[RR %p] connectivity changed for subchannel %p, subchannel_list %p " + "(index %" PRIuPTR " of %" PRIuPTR "): prev_state=%s new_state=%s", + p, subchannel(), subchannel_list(), Index(), + subchannel_list()->num_subchannels(), + (logical_connectivity_state_.has_value() + ? ConnectivityStateName(*logical_connectivity_state_) + : "N/A"), + ConnectivityStateName(connectivity_state)); + } + // Decide what state to report for aggregation purposes. + // If the last logical state was TRANSIENT_FAILURE, then ignore the + // state change unless the new state is READY. + if (logical_connectivity_state_.has_value() && + *logical_connectivity_state_ == GRPC_CHANNEL_TRANSIENT_FAILURE && + connectivity_state != GRPC_CHANNEL_READY) { + return; + } + // If the new state is IDLE, treat it as CONNECTING, since it will + // immediately transition into CONNECTING anyway. + if (connectivity_state == GRPC_CHANNEL_IDLE) { + if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) { + gpr_log(GPR_INFO, + "[RR %p] subchannel %p, subchannel_list %p (index %" PRIuPTR + " of %" PRIuPTR "): treating IDLE as CONNECTING", + p, subchannel(), subchannel_list(), Index(), + subchannel_list()->num_subchannels()); + } + connectivity_state = GRPC_CHANNEL_CONNECTING; + } + // If no change, return false. + if (logical_connectivity_state_.has_value() && + *logical_connectivity_state_ == connectivity_state) { + return; + } + // Otherwise, update counters and logical state. + subchannel_list()->UpdateStateCountersLocked(logical_connectivity_state_, + connectivity_state); + logical_connectivity_state_ = connectivity_state; +} + +// +// round_robin LB policy (with dualstack changes) +// + class RoundRobin : public LoadBalancingPolicy { public: explicit RoundRobin(Args args); @@ -450,6 +897,9 @@ class RoundRobinFactory : public LoadBalancingPolicyFactory { public: OrphanablePtr CreateLoadBalancingPolicy( LoadBalancingPolicy::Args args) const override { + if (!IsRoundRobinDelegateToPickFirstEnabled()) { + return MakeOrphanable(std::move(args)); + } return MakeOrphanable(std::move(args)); } diff --git a/src/core/lib/experiments/experiments.cc b/src/core/lib/experiments/experiments.cc index 7baa8b1fcaeb1..61d87841c08fa 100644 --- a/src/core/lib/experiments/experiments.cc +++ b/src/core/lib/experiments/experiments.cc @@ -105,10 +105,11 @@ const char* const description_jitter_max_idle = "only on max connection age, but it seems like this could smooth out some " "herding problems."; const char* const additional_constraints_jitter_max_idle = "{}"; -const char* const description_round_robin_dualstack = +const char* const description_round_robin_delegate_to_pick_first = "Change round_robin code to delegate to pick_first as per dualstack " "backend design."; -const char* const additional_constraints_round_robin_dualstack = "{}"; +const char* const additional_constraints_round_robin_delegate_to_pick_first = + "{}"; } // namespace namespace grpc_core { @@ -162,8 +163,9 @@ const ExperimentMetadata g_experiment_metadata[] = { additional_constraints_lazier_stream_updates, true, true}, {"jitter_max_idle", description_jitter_max_idle, additional_constraints_jitter_max_idle, true, true}, - {"round_robin_dualstack", description_round_robin_dualstack, - additional_constraints_round_robin_dualstack, true, true}, + {"round_robin_delegate_to_pick_first", + description_round_robin_delegate_to_pick_first, + additional_constraints_round_robin_delegate_to_pick_first, true, true}, }; } // namespace grpc_core @@ -253,10 +255,11 @@ const char* const description_jitter_max_idle = "only on max connection age, but it seems like this could smooth out some " "herding problems."; const char* const additional_constraints_jitter_max_idle = "{}"; -const char* const description_round_robin_dualstack = +const char* const description_round_robin_delegate_to_pick_first = "Change round_robin code to delegate to pick_first as per dualstack " "backend design."; -const char* const additional_constraints_round_robin_dualstack = "{}"; +const char* const additional_constraints_round_robin_delegate_to_pick_first = + "{}"; } // namespace namespace grpc_core { @@ -310,8 +313,9 @@ const ExperimentMetadata g_experiment_metadata[] = { additional_constraints_lazier_stream_updates, true, true}, {"jitter_max_idle", description_jitter_max_idle, additional_constraints_jitter_max_idle, true, true}, - {"round_robin_dualstack", description_round_robin_dualstack, - additional_constraints_round_robin_dualstack, true, true}, + {"round_robin_delegate_to_pick_first", + description_round_robin_delegate_to_pick_first, + additional_constraints_round_robin_delegate_to_pick_first, true, true}, }; } // namespace grpc_core @@ -401,10 +405,11 @@ const char* const description_jitter_max_idle = "only on max connection age, but it seems like this could smooth out some " "herding problems."; const char* const additional_constraints_jitter_max_idle = "{}"; -const char* const description_round_robin_dualstack = +const char* const description_round_robin_delegate_to_pick_first = "Change round_robin code to delegate to pick_first as per dualstack " "backend design."; -const char* const additional_constraints_round_robin_dualstack = "{}"; +const char* const additional_constraints_round_robin_delegate_to_pick_first = + "{}"; } // namespace namespace grpc_core { @@ -458,8 +463,9 @@ const ExperimentMetadata g_experiment_metadata[] = { additional_constraints_lazier_stream_updates, true, true}, {"jitter_max_idle", description_jitter_max_idle, additional_constraints_jitter_max_idle, true, true}, - {"round_robin_dualstack", description_round_robin_dualstack, - additional_constraints_round_robin_dualstack, true, true}, + {"round_robin_delegate_to_pick_first", + description_round_robin_delegate_to_pick_first, + additional_constraints_round_robin_delegate_to_pick_first, true, true}, }; } // namespace grpc_core diff --git a/src/core/lib/experiments/experiments.h b/src/core/lib/experiments/experiments.h index 1aee6347e8f46..f241376d9ec57 100644 --- a/src/core/lib/experiments/experiments.h +++ b/src/core/lib/experiments/experiments.h @@ -87,8 +87,8 @@ inline bool IsKeepaliveServerFixEnabled() { return false; } inline bool IsLazierStreamUpdatesEnabled() { return true; } #define GRPC_EXPERIMENT_IS_INCLUDED_JITTER_MAX_IDLE inline bool IsJitterMaxIdleEnabled() { return true; } -#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DUALSTACK -inline bool IsRoundRobinDualstackEnabled() { return true; } +#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DELEGATE_TO_PICK_FIRST +inline bool IsRoundRobinDelegateToPickFirstEnabled() { return true; } #elif defined(GPR_WINDOWS) inline bool IsTcpFrameSizeTuningEnabled() { return false; } @@ -119,8 +119,8 @@ inline bool IsKeepaliveServerFixEnabled() { return false; } inline bool IsLazierStreamUpdatesEnabled() { return true; } #define GRPC_EXPERIMENT_IS_INCLUDED_JITTER_MAX_IDLE inline bool IsJitterMaxIdleEnabled() { return true; } -#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DUALSTACK -inline bool IsRoundRobinDualstackEnabled() { return true; } +#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DELEGATE_TO_PICK_FIRST +inline bool IsRoundRobinDelegateToPickFirstEnabled() { return true; } #else inline bool IsTcpFrameSizeTuningEnabled() { return false; } @@ -151,8 +151,8 @@ inline bool IsKeepaliveServerFixEnabled() { return false; } inline bool IsLazierStreamUpdatesEnabled() { return true; } #define GRPC_EXPERIMENT_IS_INCLUDED_JITTER_MAX_IDLE inline bool IsJitterMaxIdleEnabled() { return true; } -#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DUALSTACK -inline bool IsRoundRobinDualstackEnabled() { return true; } +#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DELEGATE_TO_PICK_FIRST +inline bool IsRoundRobinDelegateToPickFirstEnabled() { return true; } #endif #else @@ -208,8 +208,10 @@ inline bool IsKeepaliveServerFixEnabled() { return IsExperimentEnabled(20); } inline bool IsLazierStreamUpdatesEnabled() { return IsExperimentEnabled(21); } #define GRPC_EXPERIMENT_IS_INCLUDED_JITTER_MAX_IDLE inline bool IsJitterMaxIdleEnabled() { return IsExperimentEnabled(22); } -#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DUALSTACK -inline bool IsRoundRobinDualstackEnabled() { return IsExperimentEnabled(23); } +#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DELEGATE_TO_PICK_FIRST +inline bool IsRoundRobinDelegateToPickFirstEnabled() { + return IsExperimentEnabled(23); +} constexpr const size_t kNumExperiments = 24; extern const ExperimentMetadata g_experiment_metadata[kNumExperiments]; diff --git a/src/core/lib/experiments/experiments.yaml b/src/core/lib/experiments/experiments.yaml index 45330706aa8a8..992a3e99415ee 100644 --- a/src/core/lib/experiments/experiments.yaml +++ b/src/core/lib/experiments/experiments.yaml @@ -185,10 +185,10 @@ owner: ctiller@google.com test_tags: [] allow_in_fuzzing_config: true -- name: round_robin_dualstack +- name: round_robin_delegate_to_pick_first description: Change round_robin code to delegate to pick_first as per dualstack backend design. expiry: 2023/11/15 owner: roth@google.com - test_tags: [] + test_tags: ["cpp_lb_end2end_test", "xds_end2end_test"] diff --git a/src/core/lib/experiments/rollouts.yaml b/src/core/lib/experiments/rollouts.yaml index 7cac2d618501b..f1770b22fae88 100644 --- a/src/core/lib/experiments/rollouts.yaml +++ b/src/core/lib/experiments/rollouts.yaml @@ -94,5 +94,5 @@ default: true - name: jitter_max_idle default: true -- name: round_robin_dualstack +- name: round_robin_delegate_to_pick_first default: true diff --git a/test/cpp/end2end/BUILD b/test/cpp/end2end/BUILD index 69b01b60d64b4..197f5126b55e1 100644 --- a/test/cpp/end2end/BUILD +++ b/test/cpp/end2end/BUILD @@ -513,6 +513,7 @@ grpc_cc_test( flaky = True, # TODO(b/151315347) tags = [ "cpp_end2end_test", + "cpp_lb_end2end_test", "no_windows", ], # TODO(jtattermusch): fix test on windows deps = [ @@ -597,6 +598,7 @@ grpc_cc_test( flaky = True, # TODO(b/150567713) tags = [ "cpp_end2end_test", + "cpp_lb_end2end_test", "no_windows", ], # TODO(jtattermusch): fix test on windows deps = [