From 6a2bec7fcdb1164bf338e5867b55421d9c492958 Mon Sep 17 00:00:00 2001 From: Hope Casey-Allen Date: Fri, 19 Jul 2019 11:30:39 -0700 Subject: [PATCH] Channelz: track listen sockets in the server node --- src/core/lib/channel/channelz.cc | 24 ++++++++++++++++-------- src/core/lib/channel/channelz.h | 19 ++++++++++++------- src/core/lib/surface/server.cc | 15 ++++++--------- src/core/lib/surface/server.h | 4 ---- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index b9f65870cfe..5aabedac043 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -303,9 +303,7 @@ void ChannelNode::RemoveChildSubchannel(intptr_t child_uuid) { // ServerNode::ServerNode(grpc_server* server, size_t channel_tracer_max_nodes) - : BaseNode(EntityType::kServer), - server_(server), - trace_(channel_tracer_max_nodes) {} + : BaseNode(EntityType::kServer), trace_(channel_tracer_max_nodes) {} ServerNode::~ServerNode() {} @@ -319,6 +317,16 @@ void ServerNode::RemoveChildSocket(intptr_t child_uuid) { child_sockets_.erase(child_uuid); } +void ServerNode::AddChildListenSocket(intptr_t child_uuid) { + MutexLock lock(&child_mu_); + child_listen_sockets_.insert(MakePair(child_uuid, true)); +} + +void ServerNode::RemoveChildListenSocket(intptr_t child_uuid) { + MutexLock lock(&child_mu_); + child_listen_sockets_.erase(child_uuid); +} + char* ServerNode::RenderServerSockets(intptr_t start_socket_id, intptr_t max_results) { // If user does not set max_results, we choose 500. @@ -382,17 +390,17 @@ grpc_json* ServerNode::RenderJson() { // ask CallCountingHelper to populate trace and call count data. call_counter_.PopulateCallCounts(json); json = top_level_json; - ChildRefsList listen_sockets; - grpc_server_populate_listen_sockets(server_, &listen_sockets); - if (!listen_sockets.empty()) { + // Render listen sockets + MutexLock lock(&child_mu_); + if (!child_listen_sockets_.empty()) { grpc_json* array_parent = grpc_json_create_child( nullptr, json, "listenSocket", nullptr, GRPC_JSON_ARRAY, false); - for (size_t i = 0; i < listen_sockets.size(); ++i) { + for (const auto& it : child_listen_sockets_) { json_iterator = grpc_json_create_child(json_iterator, array_parent, nullptr, nullptr, GRPC_JSON_OBJECT, false); grpc_json_add_number_string_child(json_iterator, nullptr, "socketId", - listen_sockets[i]); + it.first); } } return top_level_json; diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index a02cb82cc75..4f4dc76f8ef 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -58,11 +58,6 @@ namespace channelz { grpc_arg MakeParentUuidArg(intptr_t parent_uuid); intptr_t GetParentUuidFromArgs(const grpc_channel_args& args); -// TODO(ncteisen), this only contains the uuids of the children for now, -// since that is all that is strictly needed. In a future enhancement we will -// add human readable names as in the channelz.proto -typedef InlinedVector ChildRefsList; - class SocketNode; namespace testing { @@ -217,6 +212,13 @@ class ServerNode : public BaseNode { void RemoveChildSocket(intptr_t child_uuid); + // TODO(ncteisen): This only takes in the uuid of the child socket for now, + // since that is all that is strictly needed. In a future enhancement we will + // add human readable names as in the channelz.proto + void AddChildListenSocket(intptr_t child_uuid); + + void RemoveChildListenSocket(intptr_t child_uuid); + // proxy methods to composed classes. void AddTraceEvent(ChannelTrace::Severity severity, const grpc_slice& data) { trace_.AddTraceEvent(severity, data); @@ -232,11 +234,14 @@ class ServerNode : public BaseNode { void RecordCallSucceeded() { call_counter_.RecordCallSucceeded(); } private: - grpc_server* server_; CallCountingHelper call_counter_; ChannelTrace trace_; - Mutex child_mu_; // Guards child map below. + Mutex child_mu_; // Guards child maps below. Map> child_sockets_; + // TODO(roth): We don't actually use the values here, only the keys, so + // these should be sets instead of maps, but we don't currently have a set + // implementation. Change this if/when we have one. + Map child_listen_sockets_; }; // Handles channelz bookkeeping for sockets diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index a1c7d132ade..ad929e1d639 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -1252,15 +1252,6 @@ void grpc_server_setup_transport( grpc_transport_perform_op(transport, op); } -void grpc_server_populate_listen_sockets( - grpc_server* server, grpc_core::channelz::ChildRefsList* listen_sockets) { - gpr_mu_lock(&server->mu_global); - for (listener* l = server->listeners; l != nullptr; l = l->next) { - listen_sockets->push_back(l->socket_uuid); - } - gpr_mu_unlock(&server->mu_global); -} - void done_published_shutdown(void* done_arg, grpc_cq_completion* storage) { (void)done_arg; gpr_free(storage); @@ -1348,6 +1339,9 @@ void grpc_server_shutdown_and_notify(grpc_server* server, GRPC_CLOSURE_INIT(&l->destroy_done, listener_destroy_done, server, grpc_schedule_on_exec_ctx); l->destroy(server, l->arg, &l->destroy_done); + if (server->channelz_server != nullptr && l->socket_uuid != 0) { + server->channelz_server->RemoveChildListenSocket(l->socket_uuid); + } } channel_broadcaster_shutdown(&broadcaster, true /* send_goaway */, @@ -1411,6 +1405,9 @@ void grpc_server_add_listener(grpc_server* server, void* arg, l->start = start; l->destroy = destroy; l->socket_uuid = socket_uuid; + if (server->channelz_server != nullptr && socket_uuid != 0) { + server->channelz_server->AddChildListenSocket(socket_uuid); + } l->next = server->listeners; server->listeners = l; } diff --git a/src/core/lib/surface/server.h b/src/core/lib/surface/server.h index 926a5825006..67ad0310a96 100644 --- a/src/core/lib/surface/server.h +++ b/src/core/lib/surface/server.h @@ -51,10 +51,6 @@ void grpc_server_setup_transport( socket_node, grpc_resource_user* resource_user = nullptr); -/* fills in the uuids of all listen sockets on this server */ -void grpc_server_populate_listen_sockets( - grpc_server* server, grpc_core::channelz::ChildRefsList* listen_sockets); - grpc_core::channelz::ServerNode* grpc_server_get_channelz_node( grpc_server* server);