From b03dffbfefcf74aecff7833418af5d04cf00778f Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 7 Feb 2024 16:32:44 +0000 Subject: [PATCH] [client channel] remove grpc_channel_num_external_connectivity_watchers() --- include/grpc/grpc.h | 6 ------ .../client_channel/channel_connectivity.cc | 15 --------------- .../ext/filters/client_channel/client_channel.h | 5 ----- test/core/surface/concurrent_connectivity_test.cc | 4 ---- .../num_external_connectivity_watchers_test.cc | 14 +++----------- test/core/surface/sequential_connectivity_test.cc | 3 --- 6 files changed, 3 insertions(+), 44 deletions(-) diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index d03f8a7df0c..ad475897b17 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -177,12 +177,6 @@ GRPCAPI int grpc_completion_queue_thread_local_cache_flush( GRPCAPI grpc_connectivity_state grpc_channel_check_connectivity_state( grpc_channel* channel, int try_to_connect); -/** Number of active "external connectivity state watchers" attached to a - * channel. - * Useful for testing. **/ -GRPCAPI int grpc_channel_num_external_connectivity_watchers( - grpc_channel* channel); - /** Watch for a change in connectivity state. Once the channel connectivity state is different from last_observed_state, tag will be enqueued on cq with success=1. diff --git a/src/core/ext/filters/client_channel/channel_connectivity.cc b/src/core/ext/filters/client_channel/channel_connectivity.cc index d216c98b1fb..8795a1faae3 100644 --- a/src/core/ext/filters/client_channel/channel_connectivity.cc +++ b/src/core/ext/filters/client_channel/channel_connectivity.cc @@ -81,21 +81,6 @@ grpc_connectivity_state grpc_channel_check_connectivity_state( return client_channel->CheckConnectivityState(try_to_connect); } -int grpc_channel_num_external_connectivity_watchers(grpc_channel* c_channel) { - grpc_core::Channel* channel = grpc_core::Channel::FromC(c_channel); - grpc_core::ClientChannelFilter* client_channel = - grpc_core::ClientChannelFilter::GetFromChannel(channel); - if (client_channel == nullptr) { - if (!grpc_core::IsLameChannel(channel)) { - gpr_log(GPR_ERROR, - "grpc_channel_num_external_connectivity_watchers called on " - "something that is not a client channel"); - } - return 0; - } - return client_channel->NumExternalConnectivityWatchers(); -} - int grpc_channel_support_connectivity_watcher(grpc_channel* channel) { return grpc_core::ClientChannelFilter::GetFromChannel( grpc_core::Channel::FromC(channel)) != nullptr; diff --git a/src/core/ext/filters/client_channel/client_channel.h b/src/core/ext/filters/client_channel/client_channel.h index 6da29141bd3..6e9cec84b4f 100644 --- a/src/core/ext/filters/client_channel/client_channel.h +++ b/src/core/ext/filters/client_channel/client_channel.h @@ -149,11 +149,6 @@ class ClientChannelFilter { this, on_complete, /*cancel=*/true); } - int NumExternalConnectivityWatchers() const { - MutexLock lock(&external_watchers_mu_); - return static_cast(external_watchers_.size()); - } - // Starts and stops a connectivity watch. The watcher will be initially // notified as soon as the state changes from initial_state and then on // every subsequent state change until either the watch is stopped or diff --git a/test/core/surface/concurrent_connectivity_test.cc b/test/core/surface/concurrent_connectivity_test.cc index abf0ac7cd22..2b1ee72dc94 100644 --- a/test/core/surface/concurrent_connectivity_test.cc +++ b/test/core/surface/concurrent_connectivity_test.cc @@ -93,8 +93,6 @@ void create_loop_destroy(void* addr) { grpc_timeout_milliseconds_to_deadline(POLL_MILLIS); ASSERT_EQ(grpc_completion_queue_next(cq, poll_time, nullptr).type, GRPC_OP_COMPLETE); - // check that the watcher from "watch state" was free'd - ASSERT_EQ(grpc_channel_num_external_connectivity_watchers(chan), 0); } grpc_channel_destroy(chan); grpc_completion_queue_destroy(cq); @@ -292,8 +290,6 @@ void watches_with_short_timeouts(void* addr) { grpc_event ev = grpc_completion_queue_next(cq, poll_time, nullptr); ASSERT_EQ(ev.type, GRPC_OP_COMPLETE); ASSERT_EQ(ev.success, false); - // check that the watcher from "watch state" was free'd - ASSERT_EQ(grpc_channel_num_external_connectivity_watchers(chan), 0); } grpc_channel_destroy(chan); grpc_completion_queue_destroy(cq); diff --git a/test/core/surface/num_external_connectivity_watchers_test.cc b/test/core/surface/num_external_connectivity_watchers_test.cc index 5917330aaa0..1d72321b769 100644 --- a/test/core/surface/num_external_connectivity_watchers_test.cc +++ b/test/core/surface/num_external_connectivity_watchers_test.cc @@ -55,8 +55,6 @@ static void channel_idle_start_watch(grpc_channel* channel, grpc_channel_watch_connectivity_state(channel, GRPC_CHANNEL_IDLE, connect_deadline, cq, reinterpret_cast(next_tag++)); - gpr_log(GPR_DEBUG, "number of active connect watchers: %d", - grpc_channel_num_external_connectivity_watchers(channel)); } static void channel_idle_poll_for_timeout(grpc_channel* channel, @@ -71,9 +69,8 @@ static void channel_idle_poll_for_timeout(grpc_channel* channel, GRPC_CHANNEL_IDLE); } -// Test and use the "num_external_watchers" call to make sure -// that "connectivity watcher" structs are free'd just after, if -// their corresponding timeouts occur. +// Test to make sure that "connectivity watcher" structs are free'd just +// after, if their corresponding timeouts occur. static void run_timeouts_test(const test_fixture* fixture) { gpr_log(GPR_INFO, "TEST: %s", fixture->name); @@ -87,7 +84,6 @@ static void run_timeouts_test(const test_fixture* fixture) { // start 1 watcher and then let it time out channel_idle_start_watch(channel, cq); channel_idle_poll_for_timeout(channel, cq); - ASSERT_EQ(grpc_channel_num_external_connectivity_watchers(channel), 0); // start 3 watchers and then let them all time out for (size_t i = 1; i <= 3; i++) { @@ -96,7 +92,6 @@ static void run_timeouts_test(const test_fixture* fixture) { for (size_t i = 1; i <= 3; i++) { channel_idle_poll_for_timeout(channel, cq); } - ASSERT_EQ(grpc_channel_num_external_connectivity_watchers(channel), 0); // start 3 watchers, see one time out, start another 3, and then see them all // time out @@ -110,7 +105,6 @@ static void run_timeouts_test(const test_fixture* fixture) { for (size_t i = 1; i <= 5; i++) { channel_idle_poll_for_timeout(channel, cq); } - ASSERT_EQ(grpc_channel_num_external_connectivity_watchers(channel), 0); grpc_channel_destroy(channel); grpc_completion_queue_shutdown(cq); @@ -136,9 +130,7 @@ static void run_channel_shutdown_before_timeout_test( grpc_channel* channel = fixture->create_channel(addr.c_str()); grpc_completion_queue* cq = grpc_completion_queue_create_for_next(nullptr); - // start 1 watcher and then shut down the channel before the timer goes off - ASSERT_EQ(grpc_channel_num_external_connectivity_watchers(channel), 0); - + // start 1 watcher and then shut down the channel before the timer goes off. // expecting a 30 second timeout to go off much later than the shutdown. gpr_timespec connect_deadline = grpc_timeout_seconds_to_deadline(30); ASSERT_EQ(grpc_channel_check_connectivity_state(channel, 0), diff --git a/test/core/surface/sequential_connectivity_test.cc b/test/core/surface/sequential_connectivity_test.cc index d9623f22cee..bba66377a22 100644 --- a/test/core/surface/sequential_connectivity_test.cc +++ b/test/core/surface/sequential_connectivity_test.cc @@ -124,9 +124,6 @@ static void run_test(const test_fixture* fixture, bool share_subchannel) { connect_deadline, cq, nullptr); grpc_event ev = grpc_completion_queue_next( cq, gpr_inf_future(GPR_CLOCK_REALTIME), nullptr); - // check that the watcher from "watch state" was free'd - ASSERT_EQ(grpc_channel_num_external_connectivity_watchers(channels[i]), - 0); ASSERT_EQ(ev.type, GRPC_OP_COMPLETE); ASSERT_EQ(ev.tag, nullptr); ASSERT_EQ(ev.success, true);