[pick first] remove happy eyeballs experiment (#36092)

Closes #36092

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36092 from markdroth:pf_happy_eyeballs_cleanup 24d49849bb
PiperOrigin-RevId: 615157870
pull/35135/head
Mark D. Roth 1 year ago committed by Copybara-Service
parent 57ff0b890f
commit 6546fcd196
  1. 10
      bazel/experiments.bzl
  2. 1
      src/core/BUILD
  3. 15
      src/core/lib/experiments/experiments.cc
  4. 11
      src/core/lib/experiments/experiments.h
  5. 6
      src/core/lib/experiments/experiments.yaml
  6. 2
      src/core/lib/experiments/rollouts.yaml
  7. 160
      src/core/load_balancing/pick_first/pick_first.cc
  8. 6
      test/core/client_channel/lb_policy/pick_first_test.cc

@ -33,7 +33,6 @@ EXPERIMENT_ENABLES = {
"multiping": "multiping",
"peer_state_based_framing": "peer_state_based_framing",
"pending_queue_cap": "pending_queue_cap",
"pick_first_happy_eyeballs": "pick_first_happy_eyeballs",
"promise_based_client_call": "event_engine_client,event_engine_listener,promise_based_client_call",
"promise_based_server_call": "promise_based_server_call",
"chaotic_good": "chaotic_good,event_engine_client,event_engine_listener,promise_based_client_call,promise_based_server_call",
@ -103,7 +102,6 @@ EXPERIMENTS = {
"event_engine_listener",
],
"cpp_lb_end2end_test": [
"pick_first_happy_eyeballs",
"round_robin_delegate_to_pick_first",
"wrr_delegate_to_pick_first",
],
@ -114,7 +112,6 @@ EXPERIMENTS = {
"event_engine_listener",
],
"lb_unit_test": [
"pick_first_happy_eyeballs",
"round_robin_delegate_to_pick_first",
"wrr_delegate_to_pick_first",
],
@ -122,7 +119,6 @@ EXPERIMENTS = {
"registered_method_lookup_in_transport",
],
"xds_end2end_test": [
"pick_first_happy_eyeballs",
"round_robin_delegate_to_pick_first",
"wrr_delegate_to_pick_first",
],
@ -165,7 +161,6 @@ EXPERIMENTS = {
},
"on": {
"cpp_lb_end2end_test": [
"pick_first_happy_eyeballs",
"round_robin_delegate_to_pick_first",
"wrr_delegate_to_pick_first",
],
@ -173,7 +168,6 @@ EXPERIMENTS = {
"absl_base64",
],
"lb_unit_test": [
"pick_first_happy_eyeballs",
"round_robin_delegate_to_pick_first",
"wrr_delegate_to_pick_first",
],
@ -181,7 +175,6 @@ EXPERIMENTS = {
"registered_method_lookup_in_transport",
],
"xds_end2end_test": [
"pick_first_happy_eyeballs",
"round_robin_delegate_to_pick_first",
"wrr_delegate_to_pick_first",
],
@ -243,7 +236,6 @@ EXPERIMENTS = {
"work_serializer_dispatch",
],
"cpp_lb_end2end_test": [
"pick_first_happy_eyeballs",
"round_robin_delegate_to_pick_first",
"wrr_delegate_to_pick_first",
],
@ -254,7 +246,6 @@ EXPERIMENTS = {
"event_engine_listener",
],
"lb_unit_test": [
"pick_first_happy_eyeballs",
"round_robin_delegate_to_pick_first",
"work_serializer_dispatch",
"wrr_delegate_to_pick_first",
@ -266,7 +257,6 @@ EXPERIMENTS = {
"registered_method_lookup_in_transport",
],
"xds_end2end_test": [
"pick_first_happy_eyeballs",
"round_robin_delegate_to_pick_first",
"work_serializer_dispatch",
"wrr_delegate_to_pick_first",

@ -5555,7 +5555,6 @@ grpc_cc_library(
deps = [
"channel_args",
"connectivity_state",
"experiments",
"health_check_client",
"iomgr_fwd",
"json",

@ -80,9 +80,6 @@ const char* const description_pending_queue_cap =
"grpc_server_request_call or grpc_server_request_registered_call (or their "
"wrappers in the C++ API).";
const char* const additional_constraints_pending_queue_cap = "{}";
const char* const description_pick_first_happy_eyeballs =
"Use Happy Eyeballs in pick_first.";
const char* const additional_constraints_pick_first_happy_eyeballs = "{}";
const char* const description_promise_based_client_call =
"If set, use the new gRPC promise based call code when it's appropriate "
"(ie when all filters in a stack are promise based)";
@ -215,8 +212,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
additional_constraints_peer_state_based_framing, nullptr, 0, false, true},
{"pending_queue_cap", description_pending_queue_cap,
additional_constraints_pending_queue_cap, nullptr, 0, true, true},
{"pick_first_happy_eyeballs", description_pick_first_happy_eyeballs,
additional_constraints_pick_first_happy_eyeballs, nullptr, 0, true, true},
{"promise_based_client_call", description_promise_based_client_call,
additional_constraints_promise_based_client_call,
required_experiments_promise_based_client_call, 2, false, true},
@ -334,9 +329,6 @@ const char* const description_pending_queue_cap =
"grpc_server_request_call or grpc_server_request_registered_call (or their "
"wrappers in the C++ API).";
const char* const additional_constraints_pending_queue_cap = "{}";
const char* const description_pick_first_happy_eyeballs =
"Use Happy Eyeballs in pick_first.";
const char* const additional_constraints_pick_first_happy_eyeballs = "{}";
const char* const description_promise_based_client_call =
"If set, use the new gRPC promise based call code when it's appropriate "
"(ie when all filters in a stack are promise based)";
@ -469,8 +461,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
additional_constraints_peer_state_based_framing, nullptr, 0, false, true},
{"pending_queue_cap", description_pending_queue_cap,
additional_constraints_pending_queue_cap, nullptr, 0, true, true},
{"pick_first_happy_eyeballs", description_pick_first_happy_eyeballs,
additional_constraints_pick_first_happy_eyeballs, nullptr, 0, true, true},
{"promise_based_client_call", description_promise_based_client_call,
additional_constraints_promise_based_client_call,
required_experiments_promise_based_client_call, 2, false, true},
@ -588,9 +578,6 @@ const char* const description_pending_queue_cap =
"grpc_server_request_call or grpc_server_request_registered_call (or their "
"wrappers in the C++ API).";
const char* const additional_constraints_pending_queue_cap = "{}";
const char* const description_pick_first_happy_eyeballs =
"Use Happy Eyeballs in pick_first.";
const char* const additional_constraints_pick_first_happy_eyeballs = "{}";
const char* const description_promise_based_client_call =
"If set, use the new gRPC promise based call code when it's appropriate "
"(ie when all filters in a stack are promise based)";
@ -723,8 +710,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
additional_constraints_peer_state_based_framing, nullptr, 0, false, true},
{"pending_queue_cap", description_pending_queue_cap,
additional_constraints_pending_queue_cap, nullptr, 0, true, true},
{"pick_first_happy_eyeballs", description_pick_first_happy_eyeballs,
additional_constraints_pick_first_happy_eyeballs, nullptr, 0, true, true},
{"promise_based_client_call", description_promise_based_client_call,
additional_constraints_promise_based_client_call,
required_experiments_promise_based_client_call, 2, false, true},

@ -86,8 +86,6 @@ inline bool IsMultipingEnabled() { return false; }
inline bool IsPeerStateBasedFramingEnabled() { return false; }
#define GRPC_EXPERIMENT_IS_INCLUDED_PENDING_QUEUE_CAP
inline bool IsPendingQueueCapEnabled() { return true; }
#define GRPC_EXPERIMENT_IS_INCLUDED_PICK_FIRST_HAPPY_EYEBALLS
inline bool IsPickFirstHappyEyeballsEnabled() { return true; }
inline bool IsPromiseBasedClientCallEnabled() { return false; }
inline bool IsPromiseBasedServerCallEnabled() { return false; }
inline bool IsChaoticGoodEnabled() { return false; }
@ -144,8 +142,6 @@ inline bool IsMultipingEnabled() { return false; }
inline bool IsPeerStateBasedFramingEnabled() { return false; }
#define GRPC_EXPERIMENT_IS_INCLUDED_PENDING_QUEUE_CAP
inline bool IsPendingQueueCapEnabled() { return true; }
#define GRPC_EXPERIMENT_IS_INCLUDED_PICK_FIRST_HAPPY_EYEBALLS
inline bool IsPickFirstHappyEyeballsEnabled() { return true; }
inline bool IsPromiseBasedClientCallEnabled() { return false; }
inline bool IsPromiseBasedServerCallEnabled() { return false; }
inline bool IsChaoticGoodEnabled() { return false; }
@ -203,8 +199,6 @@ inline bool IsMultipingEnabled() { return false; }
inline bool IsPeerStateBasedFramingEnabled() { return false; }
#define GRPC_EXPERIMENT_IS_INCLUDED_PENDING_QUEUE_CAP
inline bool IsPendingQueueCapEnabled() { return true; }
#define GRPC_EXPERIMENT_IS_INCLUDED_PICK_FIRST_HAPPY_EYEBALLS
inline bool IsPickFirstHappyEyeballsEnabled() { return true; }
inline bool IsPromiseBasedClientCallEnabled() { return false; }
inline bool IsPromiseBasedServerCallEnabled() { return false; }
inline bool IsChaoticGoodEnabled() { return false; }
@ -250,7 +244,6 @@ enum ExperimentIds {
kExperimentIdMultiping,
kExperimentIdPeerStateBasedFraming,
kExperimentIdPendingQueueCap,
kExperimentIdPickFirstHappyEyeballs,
kExperimentIdPromiseBasedClientCall,
kExperimentIdPromiseBasedServerCall,
kExperimentIdChaoticGood,
@ -337,10 +330,6 @@ inline bool IsPeerStateBasedFramingEnabled() {
inline bool IsPendingQueueCapEnabled() {
return IsExperimentEnabled(kExperimentIdPendingQueueCap);
}
#define GRPC_EXPERIMENT_IS_INCLUDED_PICK_FIRST_HAPPY_EYEBALLS
inline bool IsPickFirstHappyEyeballsEnabled() {
return IsExperimentEnabled(kExperimentIdPickFirstHappyEyeballs);
}
#define GRPC_EXPERIMENT_IS_INCLUDED_PROMISE_BASED_CLIENT_CALL
inline bool IsPromiseBasedClientCallEnabled() {
return IsExperimentEnabled(kExperimentIdPromiseBasedClientCall);

@ -155,12 +155,6 @@
expiry: 2024/05/05
owner: ctiller@google.com
test_tags: []
- name: pick_first_happy_eyeballs
description:
Use Happy Eyeballs in pick_first.
expiry: 2024/03/15
owner: roth@google.com
test_tags: ["lb_unit_test", "cpp_lb_end2end_test", "xds_end2end_test"]
- name: promise_based_client_call
description:
If set, use the new gRPC promise based call code when it's appropriate

@ -91,8 +91,6 @@
default: false
- name: pending_queue_cap
default: true
- name: pick_first_happy_eyeballs
default: true
- name: promise_based_client_call
default:
ios: broken

@ -47,7 +47,6 @@
#include "src/core/lib/channel/metrics.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/gpr/useful.h"
#include "src/core/lib/gprpp/crash.h"
#include "src/core/lib/gprpp/debug_location.h"
@ -198,10 +197,6 @@ class PickFirst : public LoadBalancingPolicy {
// subchannel.
void ProcessUnselectedReadyLocked();
// Reacts to the current connectivity state while trying to connect.
// TODO(roth): Remove this when we remove the Happy Eyeballs experiment.
void ReactToConnectivityStateLocked();
// Backpointer to owning subchannel list. Not owned.
SubchannelList* subchannel_list_;
const size_t index_;
@ -274,9 +269,6 @@ class PickFirst : public LoadBalancingPolicy {
// finished processing.
bool shutting_down_ = false;
// TODO(roth): Remove this when we remove the Happy Eyeballs experiment.
bool in_transient_failure_ = false;
size_t num_subchannels_seen_initial_notification_ = 0;
// The index into subchannels_ to which we are currently attempting
@ -533,34 +525,30 @@ absl::Status PickFirst::UpdateLocked(UpdateArgs args) {
for (const auto& endpoint : endpoints) {
for (const auto& address : endpoint.addresses()) {
flattened_endpoints.emplace_back(address, endpoint.args());
if (IsPickFirstHappyEyeballsEnabled()) {
absl::string_view scheme = GetAddressFamily(address);
bool inserted = address_families.insert(scheme).second;
if (inserted) {
address_family_order.emplace_back(scheme,
flattened_endpoints.size() - 1);
}
absl::string_view scheme = GetAddressFamily(address);
bool inserted = address_families.insert(scheme).second;
if (inserted) {
address_family_order.emplace_back(scheme,
flattened_endpoints.size() - 1);
}
}
}
endpoints = std::move(flattened_endpoints);
// Interleave addresses as per RFC-8305 section 4.
if (IsPickFirstHappyEyeballsEnabled()) {
EndpointAddressesList interleaved_endpoints;
interleaved_endpoints.reserve(endpoints.size());
std::vector<bool> endpoints_moved(endpoints.size());
size_t scheme_index = 0;
for (size_t i = 0; i < endpoints.size(); ++i) {
EndpointAddresses* endpoint;
do {
auto& iterator = address_family_order[scheme_index++ %
address_family_order.size()];
endpoint = iterator.Next(endpoints, &endpoints_moved);
} while (endpoint == nullptr);
interleaved_endpoints.emplace_back(std::move(*endpoint));
}
endpoints = std::move(interleaved_endpoints);
EndpointAddressesList interleaved_endpoints;
interleaved_endpoints.reserve(endpoints.size());
std::vector<bool> endpoints_moved(endpoints.size());
size_t scheme_index = 0;
for (size_t i = 0; i < endpoints.size(); ++i) {
EndpointAddresses* endpoint;
do {
auto& iterator = address_family_order[scheme_index++ %
address_family_order.size()];
endpoint = iterator.Next(endpoints, &endpoints_moved);
} while (endpoint == nullptr);
interleaved_endpoints.emplace_back(std::move(*endpoint));
}
endpoints = std::move(interleaved_endpoints);
args.addresses =
std::make_shared<EndpointAddressesListIterator>(std::move(endpoints));
}
@ -738,9 +726,7 @@ void PickFirst::SubchannelList::SubchannelData::OnConnectivityStateChange(
p->UnsetSelectedSubchannel();
p->subchannel_list_ = std::move(p->latest_pending_subchannel_list_);
// Set our state to that of the pending subchannel list.
if (IsPickFirstHappyEyeballsEnabled()
? p->subchannel_list_->IsHappyEyeballsPassComplete()
: p->subchannel_list_->in_transient_failure_) {
if (p->subchannel_list_->IsHappyEyeballsPassComplete()) {
status = absl::UnavailableError(absl::StrCat(
"selected subchannel failed; switching to pending update; "
"last failure: ",
@ -772,9 +758,6 @@ void PickFirst::SubchannelList::SubchannelData::OnConnectivityStateChange(
// select in place of the current one.
// If the subchannel is READY, use it.
if (new_state == GRPC_CHANNEL_READY) {
if (!IsPickFirstHappyEyeballsEnabled()) {
subchannel_list_->in_transient_failure_ = false;
}
// We consider it a successful connection attempt only if the
// previous state was CONNECTING. In particular, we don't want to
// increment this counter if we got a new address list and found the
@ -807,10 +790,6 @@ void PickFirst::SubchannelList::SubchannelData::OnConnectivityStateChange(
// see its initial notification. Start trying to connect, starting
// with the first subchannel.
if (!old_state.has_value()) {
if (!IsPickFirstHappyEyeballsEnabled()) {
subchannel_list_->subchannels_.front().ReactToConnectivityStateLocked();
return;
}
subchannel_list_->StartConnectingNextSubchannel();
return;
}
@ -821,14 +800,6 @@ void PickFirst::SubchannelList::SubchannelData::OnConnectivityStateChange(
kMetricConnectionAttemptsFailed, 1,
{subchannel_list_->policy_->channel_control_helper()->GetTarget()}, {});
}
if (!IsPickFirstHappyEyeballsEnabled()) {
// Ignore any other updates for subchannels we're not currently trying to
// connect to.
if (index_ != subchannel_list_->attempting_index_) return;
// React to the connectivity state.
ReactToConnectivityStateLocked();
return;
}
// Otherwise, process connectivity state change.
switch (*connectivity_state_) {
case GRPC_CHANNEL_TRANSIENT_FAILURE: {
@ -898,99 +869,6 @@ void PickFirst::SubchannelList::SubchannelData::OnConnectivityStateChange(
}
}
void PickFirst::SubchannelList::SubchannelData::
ReactToConnectivityStateLocked() {
PickFirst* p = subchannel_list_->policy_.get();
// Otherwise, process connectivity state.
switch (connectivity_state_.value()) {
case GRPC_CHANNEL_READY:
// Already handled this case above, so this should not happen.
GPR_UNREACHABLE_CODE(break);
case GRPC_CHANNEL_TRANSIENT_FAILURE: {
// Find the next subchannel not in state TRANSIENT_FAILURE.
// We skip subchannels in state TRANSIENT_FAILURE to avoid a
// large recursion that could overflow the stack.
SubchannelData* found_subchannel = nullptr;
for (size_t next_index = index_ + 1;
next_index < subchannel_list_->size(); ++next_index) {
SubchannelData* sc = &subchannel_list_->subchannels_[next_index];
GPR_ASSERT(sc->connectivity_state_.has_value());
if (sc->connectivity_state_ != GRPC_CHANNEL_TRANSIENT_FAILURE) {
subchannel_list_->attempting_index_ = next_index;
found_subchannel = sc;
break;
}
}
// If we found another subchannel in the list not in state
// TRANSIENT_FAILURE, trigger the right behavior for that subchannel.
if (found_subchannel != nullptr) {
found_subchannel->ReactToConnectivityStateLocked();
break;
}
// We didn't find another subchannel not in state TRANSIENT_FAILURE,
// so report TRANSIENT_FAILURE and wait for the first subchannel
// in the list to report IDLE before continuing.
if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_pick_first_trace)) {
gpr_log(GPR_INFO,
"Pick First %p subchannel list %p failed to connect to "
"all subchannels",
p, subchannel_list_);
}
subchannel_list_->attempting_index_ = 0;
subchannel_list_->in_transient_failure_ = true;
// In case 2, swap to the new subchannel list. This means reporting
// TRANSIENT_FAILURE and dropping the existing (working) connection,
// but we can't ignore what the control plane has told us.
if (subchannel_list_ == p->latest_pending_subchannel_list_.get()) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_pick_first_trace)) {
gpr_log(GPR_INFO,
"Pick First %p promoting pending subchannel list %p to "
"replace %p",
p, p->latest_pending_subchannel_list_.get(),
p->subchannel_list_.get());
}
p->UnsetSelectedSubchannel();
p->subchannel_list_ = std::move(p->latest_pending_subchannel_list_);
}
// If this is the current subchannel list (either because we were
// in case 1 or because we were in case 2 and just promoted it to
// be the current list), re-resolve and report new state.
if (subchannel_list_ == p->subchannel_list_.get()) {
p->channel_control_helper()->RequestReresolution();
absl::Status status = absl::UnavailableError(absl::StrCat(
(p->omit_status_message_prefix_
? ""
: "failed to connect to all addresses; last error: "),
connectivity_status_.ToString()));
p->UpdateState(GRPC_CHANNEL_TRANSIENT_FAILURE, status,
MakeRefCounted<TransientFailurePicker>(status));
}
// If the first subchannel is already IDLE, trigger the next connection
// attempt immediately. Otherwise, we'll wait for it to report
// its own connectivity state change.
auto& subchannel0 = subchannel_list_->subchannels_.front();
if (subchannel0.connectivity_state_ == GRPC_CHANNEL_IDLE) {
subchannel0.subchannel_->RequestConnection();
}
break;
}
case GRPC_CHANNEL_IDLE:
subchannel_->RequestConnection();
break;
case GRPC_CHANNEL_CONNECTING:
// Only update connectivity state in case 1, and only if we're not
// already in TRANSIENT_FAILURE.
if (subchannel_list_ == p->subchannel_list_.get() &&
p->state_ != GRPC_CHANNEL_TRANSIENT_FAILURE) {
p->UpdateState(GRPC_CHANNEL_CONNECTING, absl::Status(),
MakeRefCounted<QueuePicker>(nullptr));
}
break;
case GRPC_CHANNEL_SHUTDOWN:
GPR_UNREACHABLE_CODE(break);
}
}
void PickFirst::SubchannelList::SubchannelData::RequestConnectionWithTimer() {
GPR_ASSERT(connectivity_state_.has_value());
if (connectivity_state_ == GRPC_CHANNEL_IDLE) {

@ -38,7 +38,6 @@
#include <grpc/support/json.h>
#include "src/core/lib/channel/metrics.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"
@ -513,7 +512,6 @@ TEST_F(PickFirstTest, ResolverUpdateBeforeLeavingIdle) {
}
TEST_F(PickFirstTest, HappyEyeballs) {
if (!IsPickFirstHappyEyeballsEnabled()) return;
// Send an update containing three addresses.
constexpr std::array<absl::string_view, 3> kAddresses = {
"ipv4:127.0.0.1:443", "ipv4:127.0.0.1:444", "ipv4:127.0.0.1:445"};
@ -568,7 +566,6 @@ TEST_F(PickFirstTest, HappyEyeballs) {
}
TEST_F(PickFirstTest, HappyEyeballsCompletesWithoutSuccess) {
if (!IsPickFirstHappyEyeballsEnabled()) return;
// Send an update containing three addresses.
constexpr std::array<absl::string_view, 3> kAddresses = {
"ipv4:127.0.0.1:443", "ipv4:127.0.0.1:444", "ipv4:127.0.0.1:445"};
@ -689,7 +686,6 @@ TEST_F(PickFirstTest, HappyEyeballsCompletesWithoutSuccess) {
TEST_F(PickFirstTest,
HappyEyeballsLastSubchannelFailsWhileAnotherIsStillPending) {
if (!IsPickFirstHappyEyeballsEnabled()) return;
// Send an update containing three addresses.
constexpr std::array<absl::string_view, 2> kAddresses = {
"ipv4:127.0.0.1:443", "ipv4:127.0.0.1:444"};
@ -758,7 +754,6 @@ TEST_F(PickFirstTest,
}
TEST_F(PickFirstTest, HappyEyeballsAddressInterleaving) {
if (!IsPickFirstHappyEyeballsEnabled()) return;
// Send an update containing four IPv4 addresses followed by two
// IPv6 addresses.
constexpr std::array<absl::string_view, 6> kAddresses = {
@ -851,7 +846,6 @@ TEST_F(PickFirstTest, HappyEyeballsAddressInterleaving) {
TEST_F(PickFirstTest,
HappyEyeballsAddressInterleavingSecondFamilyHasMoreAddresses) {
if (!IsPickFirstHappyEyeballsEnabled()) return;
// Send an update containing two IPv6 addresses followed by four IPv4
// addresses.
constexpr std::array<absl::string_view, 6> kAddresses = {

Loading…
Cancel
Save