diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 63bd737e931e3456ee9d96003be449630cddacb2..f88c1228de0f9406d09c548ab61005d20b78d11b 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -1345,11 +1345,11 @@ class ChannelData::ClientChannelControlHelper // No-op -- we should never get this from ResolvingLoadBalancingPolicy. void RequestReresolution() override {} - void AddTraceEvent(TraceSeverity severity, const char* message) override { + void AddTraceEvent(TraceSeverity severity, StringView message) override { if (chand_->channelz_node_ != nullptr) { chand_->channelz_node_->AddTraceEvent( ConvertSeverityEnum(severity), - grpc_slice_from_copied_string(message)); + grpc_slice_from_copied_buffer(message.data(), message.size())); } } @@ -3730,8 +3730,8 @@ const char* PickResultTypeName( return "COMPLETE"; case LoadBalancingPolicy::PickResult::PICK_QUEUE: return "QUEUE"; - case LoadBalancingPolicy::PickResult::PICK_TRANSIENT_FAILURE: - return "TRANSIENT_FAILURE"; + case LoadBalancingPolicy::PickResult::PICK_FAILED: + return "FAILED"; } GPR_UNREACHABLE_CODE(return "UNKNOWN"); } @@ -3792,7 +3792,7 @@ void CallData::StartPickLocked(void* arg, grpc_error* error) { result.subchannel.get(), grpc_error_string(result.error)); } switch (result.type) { - case LoadBalancingPolicy::PickResult::PICK_TRANSIENT_FAILURE: { + case LoadBalancingPolicy::PickResult::PICK_FAILED: { // If we're shutting down, fail all RPCs. grpc_error* disconnect_error = chand->disconnect_error(); if (disconnect_error != GRPC_ERROR_NONE) { diff --git a/src/core/ext/filters/client_channel/lb_policy.cc b/src/core/ext/filters/client_channel/lb_policy.cc index 3207f8880448969ef6369002593e7d4f28eae5dc..41a7d2d07be659d58dda1ebfe76bb32f347f069c 100644 --- a/src/core/ext/filters/client_channel/lb_policy.cc +++ b/src/core/ext/filters/client_channel/lb_policy.cc @@ -129,7 +129,7 @@ void LoadBalancingPolicy::QueuePicker::CallExitIdle(void* arg, LoadBalancingPolicy::PickResult LoadBalancingPolicy::TransientFailurePicker::Pick(PickArgs args) { PickResult result; - result.type = PickResult::PICK_TRANSIENT_FAILURE; + result.type = PickResult::PICK_FAILED; result.error = GRPC_ERROR_REF(error_); return result; } diff --git a/src/core/ext/filters/client_channel/lb_policy.h b/src/core/ext/filters/client_channel/lb_policy.h index ea4962cdb77cd2b249a651bc779af7ea456d1767..6e7447dd17218fcb6c6985a3dfd8bb55a6197c82 100644 --- a/src/core/ext/filters/client_channel/lb_policy.h +++ b/src/core/ext/filters/client_channel/lb_policy.h @@ -42,15 +42,15 @@ extern DebugOnlyTraceFlag grpc_trace_lb_policy_refcount; /// /// Channel: An abstraction that manages connections to backend servers /// on behalf of a client application. The application creates a channel -/// for a given server name and then sends RPCs on it, and the channel -/// figures out which backend server to send each RPC to. A channel +/// for a given server name and then sends calls (RPCs) on it, and the +/// channel figures out which backend server to send each call to. A channel /// contains a resolver, a load balancing policy (or a tree of LB policies), /// and a set of one or more subchannels. /// /// Subchannel: A subchannel represents a connection to one backend server. /// The LB policy decides which subchannels to create, manages the /// connectivity state of those subchannels, and decides which subchannel -/// to send any given RPC to. +/// to send any given call to. /// /// Resolver: A plugin that takes a gRPC server URI and resolves it to a /// list of one or more addresses and a service config, as described @@ -59,12 +59,12 @@ extern DebugOnlyTraceFlag grpc_trace_lb_policy_refcount; /// /// Load Balancing (LB) Policy: A plugin that takes a list of addresses /// from the resolver, maintains and manages a subchannel for each -/// backend address, and decides which subchannel to send each RPC on. +/// backend address, and decides which subchannel to send each call on. /// An LB policy has two parts: /// - A LoadBalancingPolicy, which deals with the control plane work of /// managing subchannels. /// - A SubchannelPicker, which handles the data plane work of -/// determining which subchannel a given RPC should be sent on. +/// determining which subchannel a given call should be sent on. /// LoadBalacingPolicy API. /// @@ -78,6 +78,7 @@ extern DebugOnlyTraceFlag grpc_trace_lb_policy_refcount; class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> { public: /// Interface for accessing per-call state. + /// Implemented by the client channel and used by the SubchannelPicker. class CallState { public: CallState() = default; @@ -93,6 +94,7 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> { }; /// Interface for accessing metadata. + /// Implemented by the client channel and used by the SubchannelPicker. class MetadataInterface { public: // Implementations whose iterators fit in intptr_t may internally @@ -123,7 +125,7 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> { GRPC_ABSTRACT_BASE_CLASS }; - /// Arguments used when picking a subchannel for an RPC. + /// Arguments used when picking a subchannel for a call. struct PickArgs { /// Initial metadata associated with the picking call. /// The LB policy may use the existing metadata to influence its routing @@ -135,24 +137,23 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> { CallState* call_state; }; - /// The result of picking a subchannel for an RPC. + /// The result of picking a subchannel for a call. struct PickResult { enum ResultType { - /// Pick complete. If connected_subchannel is non-null, client channel - /// can immediately proceed with the call on connected_subchannel; - /// otherwise, call should be dropped. + /// Pick complete. If \a subchannel is non-null, the client channel + /// will immediately proceed with the call on that subchannel; + /// otherwise, it will drop the call. PICK_COMPLETE, /// Pick cannot be completed until something changes on the control - /// plane. Client channel will queue the pick and try again the + /// plane. The client channel will queue the pick and try again the /// next time the picker is updated. PICK_QUEUE, - /// LB policy is in transient failure. If the pick is wait_for_ready, - /// client channel will wait for the next picker and try again; - /// otherwise, the call will be failed immediately (although it may - /// be retried if the client channel is configured to do so). - /// The Pick() method will set its error parameter if this value is - /// returned. - PICK_TRANSIENT_FAILURE, + /// Pick failed. If the call is wait_for_ready, the client channel + /// will wait for the next picker and try again; otherwise, it + /// will immediately fail the call with the status indicated via + /// \a error (although the call may be retried if the client channel + /// is configured to do so). + PICK_FAILED, }; ResultType type; @@ -160,14 +161,14 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> { /// subchannel, or nullptr if the LB policy decides to drop the call. RefCountedPtr<SubchannelInterface> subchannel; - /// Used only if type is PICK_TRANSIENT_FAILURE. - /// Error to be set when returning a transient failure. + /// Used only if type is PICK_FAILED. + /// Error to be set when returning a failure. // TODO(roth): Replace this with something similar to grpc::Status, // so that we don't expose grpc_error to this API. grpc_error* error = GRPC_ERROR_NONE; /// Used only if type is PICK_COMPLETE. - /// Callback set by lb policy to be notified of trailing metadata. + /// Callback set by LB policy to be notified of trailing metadata. /// The user_data argument will be set to the /// recv_trailing_metadata_ready_user_data field. /// recv_trailing_metadata will be set to the metadata, which may be @@ -184,11 +185,12 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> { }; /// A subchannel picker is the object used to pick the subchannel to - /// use for a given RPC. + /// use for a given call. This is implemented by the LB policy and + /// used by the client channel to perform picks. /// /// Pickers are intended to encapsulate all of the state and logic /// needed on the data plane (i.e., to actually process picks for - /// individual RPCs sent on the channel) while excluding all of the + /// individual calls sent on the channel) while excluding all of the /// state and logic needed on the control plane (i.e., resolver /// updates, connectivity state notifications, etc); the latter should /// live in the LB policy object itself. @@ -206,8 +208,8 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> { GRPC_ABSTRACT_BASE_CLASS }; - /// A proxy object used by the LB policy to communicate with the client - /// channel. + /// A proxy object implemented by the client channel and used by the + /// LB policy to communicate with the channel. // TODO(juanlishen): Consider adding a mid-layer subclass that helps handle // things like swapping in pending policy when it's ready. Currently, we are // duplicating the logic in many subclasses. @@ -235,10 +237,9 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> { virtual void RequestReresolution() GRPC_ABSTRACT; /// Adds a trace message associated with the channel. - /// Does NOT take ownership of \a message. enum TraceSeverity { TRACE_INFO, TRACE_WARNING, TRACE_ERROR }; virtual void AddTraceEvent(TraceSeverity severity, - const char* message) GRPC_ABSTRACT; + StringView message) GRPC_ABSTRACT; GRPC_ABSTRACT_BASE_CLASS }; diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 6a565e057b9e587f5151eccd812e0f87f75329af..c63717bdb92eff1b8b7f8f851890396241e45a0a 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -298,7 +298,7 @@ class GrpcLb : public LoadBalancingPolicy { void UpdateState(grpc_connectivity_state state, UniquePtr<SubchannelPicker> picker) override; void RequestReresolution() override; - void AddTraceEvent(TraceSeverity severity, const char* message) override; + void AddTraceEvent(TraceSeverity severity, StringView message) override; void set_child(LoadBalancingPolicy* child) { child_ = child; } @@ -756,8 +756,7 @@ void GrpcLb::Helper::RequestReresolution() { } } -void GrpcLb::Helper::AddTraceEvent(TraceSeverity severity, - const char* message) { +void GrpcLb::Helper::AddTraceEvent(TraceSeverity severity, StringView message) { if (parent_->shutting_down_ || (!CalledByPendingChild() && !CalledByCurrentChild())) { return; diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc index 75fa544c80a3bb187ca78de6060b2f8310aa3426..f40e58e47c568cb0306aff3d3f884d76f4205eb9 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc @@ -436,7 +436,7 @@ class XdsLb : public LoadBalancingPolicy { void UpdateState(grpc_connectivity_state state, UniquePtr<SubchannelPicker> picker) override; void RequestReresolution() override; - void AddTraceEvent(TraceSeverity severity, const char* message) override; + void AddTraceEvent(TraceSeverity severity, StringView message) override; void set_child(LoadBalancingPolicy* child) { child_ = child; } @@ -487,8 +487,7 @@ class XdsLb : public LoadBalancingPolicy { void UpdateState(grpc_connectivity_state state, UniquePtr<SubchannelPicker> picker) override; void RequestReresolution() override; - void AddTraceEvent(TraceSeverity severity, - const char* message) override; + void AddTraceEvent(TraceSeverity severity, StringView message) override; void set_child(LoadBalancingPolicy* child) { child_ = child; } private: @@ -774,7 +773,7 @@ void XdsLb::FallbackHelper::RequestReresolution() { } void XdsLb::FallbackHelper::AddTraceEvent(TraceSeverity severity, - const char* message) { + StringView message) { if (parent_->shutting_down_ || (!CalledByPendingFallback() && !CalledByCurrentFallback())) { return; @@ -2510,7 +2509,7 @@ void XdsLb::LocalityMap::LocalityEntry::Helper::RequestReresolution() { } void XdsLb::LocalityMap::LocalityEntry::Helper::AddTraceEvent( - TraceSeverity severity, const char* message) { + TraceSeverity severity, StringView message) { if (entry_->parent_->shutting_down_ || (!CalledByPendingChild() && !CalledByCurrentChild())) { return; diff --git a/src/core/ext/filters/client_channel/resolving_lb_policy.cc b/src/core/ext/filters/client_channel/resolving_lb_policy.cc index b3b455b0ce8d7b1d7a7d701aec3ecf90f7f483c6..c85e948a55bc41ba94f970ba921c602752fb1fbf 100644 --- a/src/core/ext/filters/client_channel/resolving_lb_policy.cc +++ b/src/core/ext/filters/client_channel/resolving_lb_policy.cc @@ -160,7 +160,7 @@ class ResolvingLoadBalancingPolicy::ResolvingControlHelper } } - void AddTraceEvent(TraceSeverity severity, const char* message) override {} + void AddTraceEvent(TraceSeverity severity, StringView message) override {} void set_child(LoadBalancingPolicy* child) { child_ = child; } @@ -435,7 +435,7 @@ void ResolvingLoadBalancingPolicy::ConcatenateAndAddChannelTraceLocked( size_t len = 0; UniquePtr<char> message(gpr_strvec_flatten(&v, &len)); channel_control_helper()->AddTraceEvent(ChannelControlHelper::TRACE_INFO, - message.get()); + StringView(message.get())); gpr_strvec_destroy(&v); } } diff --git a/test/core/util/test_lb_policies.cc b/test/core/util/test_lb_policies.cc index 77e186c6d499513bc05c129de52261f553e8b459..0539a49896540f0fe6d573693ac5866b3e2b05c9 100644 --- a/test/core/util/test_lb_policies.cc +++ b/test/core/util/test_lb_policies.cc @@ -166,7 +166,7 @@ class InterceptRecvTrailingMetadataLoadBalancingPolicy parent_->channel_control_helper()->RequestReresolution(); } - void AddTraceEvent(TraceSeverity severity, const char* message) override { + void AddTraceEvent(TraceSeverity severity, StringView message) override { parent_->channel_control_helper()->AddTraceEvent(severity, message); }