Merge pull request #19463 from hcaseyal/channelz_subchannel

Channelz: SubchannelNode no longer holds a pointer to the Subchannel
pull/19032/head
hcaseyal 6 years ago committed by GitHub
commit e6b29b3693
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 25
      src/core/ext/filters/client_channel/client_channel_channelz.cc
  2. 17
      src/core/ext/filters/client_channel/client_channel_channelz.h
  3. 27
      src/core/ext/filters/client_channel/subchannel.cc
  4. 8
      src/core/ext/filters/client_channel/subchannel.h

@ -30,24 +30,25 @@
namespace grpc_core {
namespace channelz {
SubchannelNode::SubchannelNode(Subchannel* subchannel,
SubchannelNode::SubchannelNode(const char* target_address,
size_t channel_tracer_max_nodes)
: BaseNode(EntityType::kSubchannel),
subchannel_(subchannel),
target_(UniquePtr<char>(gpr_strdup(subchannel_->GetTargetAddress()))),
target_(UniquePtr<char>(gpr_strdup(target_address))),
trace_(channel_tracer_max_nodes) {}
SubchannelNode::~SubchannelNode() {}
void SubchannelNode::UpdateConnectivityState(grpc_connectivity_state state) {
connectivity_state_.Store(state, MemoryOrder::RELAXED);
}
void SubchannelNode::SetChildSocketUuid(intptr_t uuid) {
child_socket_uuid_.Store(uuid, MemoryOrder::RELAXED);
}
void SubchannelNode::PopulateConnectivityState(grpc_json* json) {
grpc_connectivity_state state;
if (subchannel_ == nullptr) {
state = GRPC_CHANNEL_SHUTDOWN;
} else {
state = subchannel_->CheckConnectivityState(
nullptr /* health_check_service_name */,
nullptr /* connected_subchannel */);
}
grpc_connectivity_state state =
connectivity_state_.Load(MemoryOrder::RELAXED);
json = grpc_json_create_child(nullptr, json, "state", nullptr,
GRPC_JSON_OBJECT, false);
grpc_json_create_child(nullptr, json, "state",
@ -87,7 +88,7 @@ grpc_json* SubchannelNode::RenderJson() {
call_counter_.PopulateCallCounts(json);
json = top_level_json;
// populate the child socket.
intptr_t socket_uuid = subchannel_->GetChildSocketUuid();
intptr_t socket_uuid = child_socket_uuid_.Load(MemoryOrder::RELAXED);
if (socket_uuid != 0) {
grpc_json* array_parent = grpc_json_create_child(
nullptr, json, "socketRef", nullptr, GRPC_JSON_ARRAY, false);

@ -34,13 +34,17 @@ namespace channelz {
class SubchannelNode : public BaseNode {
public:
SubchannelNode(Subchannel* subchannel, size_t channel_tracer_max_nodes);
SubchannelNode(const char* target_address, size_t channel_tracer_max_nodes);
~SubchannelNode() override;
void MarkSubchannelDestroyed() {
GPR_ASSERT(subchannel_ != nullptr);
subchannel_ = nullptr;
}
// Sets the subchannel's connectivity state without health checking.
void UpdateConnectivityState(grpc_connectivity_state state);
// Used when the subchannel's child socket uuid changes. This should be set
// when the subchannel's transport is created and set to 0 when the subchannel
// unrefs the transport. A uuid of 0 indicates that the child socket is no
// longer associated with this subchannel.
void SetChildSocketUuid(intptr_t uuid);
grpc_json* RenderJson() override;
@ -61,7 +65,8 @@ class SubchannelNode : public BaseNode {
private:
void PopulateConnectivityState(grpc_json* json);
Subchannel* subchannel_;
Atomic<grpc_connectivity_state> connectivity_state_{GRPC_CHANNEL_IDLE};
Atomic<intptr_t> child_socket_uuid_{0};
UniquePtr<char> target_;
CallCountingHelper call_counter_;
ChannelTrace trace_;

@ -84,13 +84,11 @@ DebugOnlyTraceFlag grpc_trace_subchannel_refcount(false, "subchannel_refcount");
ConnectedSubchannel::ConnectedSubchannel(
grpc_channel_stack* channel_stack, const grpc_channel_args* args,
RefCountedPtr<channelz::SubchannelNode> channelz_subchannel,
intptr_t socket_uuid)
RefCountedPtr<channelz::SubchannelNode> channelz_subchannel)
: ConnectedSubchannelInterface(&grpc_trace_subchannel_refcount),
channel_stack_(channel_stack),
args_(grpc_channel_args_copy(args)),
channelz_subchannel_(std::move(channelz_subchannel)),
socket_uuid_(socket_uuid) {}
channelz_subchannel_(std::move(channelz_subchannel)) {}
ConnectedSubchannel::~ConnectedSubchannel() {
grpc_channel_args_destroy(args_);
@ -344,6 +342,9 @@ class Subchannel::ConnectedSubchannelStateWatcher {
self->pending_connectivity_state_));
}
c->connected_subchannel_.reset();
if (c->channelz_node() != nullptr) {
c->channelz_node()->SetChildSocketUuid(0);
}
c->SetConnectivityStateLocked(GRPC_CHANNEL_TRANSIENT_FAILURE);
c->backoff_begun_ = false;
c->backoff_.Reset();
@ -676,7 +677,7 @@ Subchannel::Subchannel(SubchannelKey* key, grpc_connector* connector,
(size_t)grpc_channel_arg_get_integer(arg, options);
if (channelz_enabled) {
channelz_node_ = MakeRefCounted<channelz::SubchannelNode>(
this, channel_tracer_max_memory);
GetTargetAddress(), channel_tracer_max_memory);
channelz_node_->AddTraceEvent(
channelz::ChannelTrace::Severity::Info,
grpc_slice_from_static_string("subchannel created"));
@ -688,7 +689,7 @@ Subchannel::~Subchannel() {
channelz_node_->AddTraceEvent(
channelz::ChannelTrace::Severity::Info,
grpc_slice_from_static_string("Subchannel destroyed"));
channelz_node_->MarkSubchannelDestroyed();
channelz_node_->UpdateConnectivityState(GRPC_CHANNEL_SHUTDOWN);
}
grpc_channel_args_destroy(args_);
grpc_connector_unref(connector_);
@ -778,14 +779,6 @@ Subchannel* Subchannel::RefFromWeakRef(GRPC_SUBCHANNEL_REF_EXTRA_ARGS) {
}
}
intptr_t Subchannel::GetChildSocketUuid() {
if (connected_subchannel_ != nullptr) {
return connected_subchannel_->socket_uuid();
} else {
return 0;
}
}
const char* Subchannel::GetTargetAddress() {
const grpc_arg* addr_arg =
grpc_channel_args_find(args_, GRPC_ARG_SUBCHANNEL_ADDRESS);
@ -930,6 +923,7 @@ const char* SubchannelConnectivityStateChangeString(
void Subchannel::SetConnectivityStateLocked(grpc_connectivity_state state) {
state_ = state;
if (channelz_node_ != nullptr) {
channelz_node_->UpdateConnectivityState(state);
channelz_node_->AddTraceEvent(
channelz::ChannelTrace::Severity::Info,
grpc_slice_from_static_string(
@ -1078,9 +1072,12 @@ bool Subchannel::PublishTransportLocked() {
}
// Publish.
connected_subchannel_.reset(
New<ConnectedSubchannel>(stk, args_, channelz_node_, socket_uuid));
New<ConnectedSubchannel>(stk, args_, channelz_node_));
gpr_log(GPR_INFO, "New connected subchannel at %p for subchannel %p",
connected_subchannel_.get(), this);
if (channelz_node_ != nullptr) {
channelz_node_->SetChildSocketUuid(socket_uuid);
}
// Instantiate state watcher. Will clean itself up.
New<ConnectedSubchannelStateWatcher>(this);
// Report initial state.

@ -85,8 +85,7 @@ class ConnectedSubchannel : public ConnectedSubchannelInterface {
ConnectedSubchannel(
grpc_channel_stack* channel_stack, const grpc_channel_args* args,
RefCountedPtr<channelz::SubchannelNode> channelz_subchannel,
intptr_t socket_uuid);
RefCountedPtr<channelz::SubchannelNode> channelz_subchannel);
~ConnectedSubchannel();
void NotifyOnStateChange(grpc_pollset_set* interested_parties,
@ -101,7 +100,6 @@ class ConnectedSubchannel : public ConnectedSubchannelInterface {
channelz::SubchannelNode* channelz_subchannel() const {
return channelz_subchannel_.get();
}
intptr_t socket_uuid() const { return socket_uuid_; }
size_t GetInitialCallSizeEstimate(size_t parent_data_size) const;
@ -111,8 +109,6 @@ class ConnectedSubchannel : public ConnectedSubchannelInterface {
// ref counted pointer to the channelz node in this connected subchannel's
// owning subchannel.
RefCountedPtr<channelz::SubchannelNode> channelz_subchannel_;
// uuid of this subchannel's socket. 0 if this subchannel is not connected.
const intptr_t socket_uuid_;
};
// Implements the interface of RefCounted<>.
@ -200,8 +196,6 @@ class Subchannel {
// returns null.
Subchannel* RefFromWeakRef(GRPC_SUBCHANNEL_REF_EXTRA_ARGS);
intptr_t GetChildSocketUuid();
// Gets the string representing the subchannel address.
// Caller doesn't take ownership.
const char* GetTargetAddress();

Loading…
Cancel
Save