From b3c0b91db18d81ddff973f2f640619819d47672d Mon Sep 17 00:00:00 2001 From: Juanli Shen Date: Tue, 15 Jan 2019 10:47:35 -0800 Subject: [PATCH] Remove force_creation param from subchannel index --- .../client_channel/subchannel_index.cc | 8 ------ .../filters/client_channel/subchannel_index.h | 9 ------- test/cpp/end2end/client_lb_end2end_test.cc | 26 +++---------------- 3 files changed, 3 insertions(+), 40 deletions(-) diff --git a/src/core/ext/filters/client_channel/subchannel_index.cc b/src/core/ext/filters/client_channel/subchannel_index.cc index d0ceda8312c..1c839ddd6a3 100644 --- a/src/core/ext/filters/client_channel/subchannel_index.cc +++ b/src/core/ext/filters/client_channel/subchannel_index.cc @@ -42,8 +42,6 @@ struct grpc_subchannel_key { grpc_channel_args* args; }; -static bool g_force_creation = false; - static grpc_subchannel_key* create_key( const grpc_channel_args* args, grpc_channel_args* (*copy_channel_args)(const grpc_channel_args* args)) { @@ -63,8 +61,6 @@ static grpc_subchannel_key* subchannel_key_copy(grpc_subchannel_key* k) { int grpc_subchannel_key_compare(const grpc_subchannel_key* a, const grpc_subchannel_key* b) { - // To pretend the keys are different, return a non-zero value. - if (GPR_UNLIKELY(g_force_creation)) return 1; return grpc_channel_args_compare(a->args, b->args); } @@ -224,7 +220,3 @@ void grpc_subchannel_index_unregister(grpc_subchannel_key* key, grpc_avl_unref(index, nullptr); } } - -void grpc_subchannel_index_test_only_set_force_creation(bool force_creation) { - g_force_creation = force_creation; -} diff --git a/src/core/ext/filters/client_channel/subchannel_index.h b/src/core/ext/filters/client_channel/subchannel_index.h index 429634bd54c..1aeb51e6535 100644 --- a/src/core/ext/filters/client_channel/subchannel_index.h +++ b/src/core/ext/filters/client_channel/subchannel_index.h @@ -63,13 +63,4 @@ void grpc_subchannel_index_ref(void); to zero, unref the subchannel index and destroy its mutex. */ void grpc_subchannel_index_unref(void); -/** \em TEST ONLY. - * If \a force_creation is true, all keys are regarded different, resulting in - * new subchannels always being created. Otherwise, the keys will be compared as - * usual. - * - * Tests using this function \em MUST run tests with and without \a - * force_creation set. */ -void grpc_subchannel_index_test_only_set_force_creation(bool force_creation); - #endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_SUBCHANNEL_INDEX_H */ diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index 929c2bb5899..9783f51ab7d 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/test/cpp/end2end/client_lb_end2end_test.cc @@ -38,7 +38,6 @@ #include "src/core/ext/filters/client_channel/parse_address.h" #include "src/core/ext/filters/client_channel/resolver/fake/fake_resolver.h" #include "src/core/ext/filters/client_channel/server_address.h" -#include "src/core/ext/filters/client_channel/subchannel_index.h" #include "src/core/lib/backoff/backoff.h" #include "src/core/lib/gpr/env.h" #include "src/core/lib/gprpp/debug_location.h" @@ -662,30 +661,14 @@ TEST_F(ClientLbEnd2endTest, PickFirstUpdateSuperset) { EXPECT_EQ("pick_first", channel->GetLoadBalancingPolicyName()); } -class ClientLbEnd2endWithParamTest - : public ClientLbEnd2endTest, - public ::testing::WithParamInterface { - protected: - void SetUp() override { - grpc_subchannel_index_test_only_set_force_creation(GetParam()); - ClientLbEnd2endTest::SetUp(); - } - - void TearDown() override { - ClientLbEnd2endTest::TearDown(); - grpc_subchannel_index_test_only_set_force_creation(false); - } -}; - -TEST_P(ClientLbEnd2endWithParamTest, PickFirstManyUpdates) { - gpr_log(GPR_INFO, "subchannel force creation: %d", GetParam()); - // Start servers and send one RPC per server. +TEST_F(ClientLbEnd2endTest, PickFirstManyUpdates) { + const int kNumUpdates = 1000; const int kNumServers = 3; StartServers(kNumServers); auto channel = BuildChannel("pick_first"); auto stub = BuildStub(channel); std::vector ports = GetServersPorts(); - for (size_t i = 0; i < 1000; ++i) { + for (size_t i = 0; i < kNumUpdates; ++i) { std::shuffle(ports.begin(), ports.end(), std::mt19937(std::random_device()())); SetNextResolution(ports); @@ -697,9 +680,6 @@ TEST_P(ClientLbEnd2endWithParamTest, PickFirstManyUpdates) { EXPECT_EQ("pick_first", channel->GetLoadBalancingPolicyName()); } -INSTANTIATE_TEST_CASE_P(SubchannelForceCreation, ClientLbEnd2endWithParamTest, - ::testing::Bool()); - TEST_F(ClientLbEnd2endTest, PickFirstReresolutionNoSelected) { // Prepare the ports for up servers and down servers. const int kNumServers = 3;