Update by review

reviewable/pr25134/r4
Esun Kim 4 years ago
parent 9bd7c4917a
commit 0db0ab052d
  1. 8
      src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc
  2. 3
      src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.cc
  3. 3
      src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h
  4. 3
      src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc
  5. 2
      src/core/ext/transport/chttp2/server/chttp2_server.cc
  6. 2
      src/core/ext/transport/chttp2/server/insecure/server_chttp2.cc
  7. 4
      src/core/ext/xds/xds_api.cc
  8. 2
      src/core/lib/security/credentials/fake/fake_credentials.cc
  9. 4
      src/core/lib/surface/server.cc
  10. 6
      src/cpp/ext/filters/census/context.cc
  11. 5
      src/cpp/ext/filters/census/context.h
  12. 13
      src/cpp/ext/filters/census/server_filter.cc
  13. 10
      test/cpp/end2end/xds_end2end_test.cc

@ -1259,12 +1259,10 @@ ServerAddressList ExtractBalancerAddresses(const grpc_channel_args& args) {
* stream for the reception of load balancing updates.
*
* Inputs:
* - \a addresses: corresponding to the balancers.
* - \a response_generator: in order to propagate updates from the resolver
* above the grpclb policy.
* - \a args: other args inherited from the grpclb policy. */
grpc_channel_args* BuildBalancerChannelArgs(
const ServerAddressList& addresses,
FakeResolverResponseGenerator* response_generator,
const grpc_channel_args* args) {
// Channel args to remove.
@ -1313,7 +1311,7 @@ grpc_channel_args* BuildBalancerChannelArgs(
args, args_to_remove, GPR_ARRAY_SIZE(args_to_remove), args_to_add.data(),
args_to_add.size());
// Make any necessary modifications for security.
return ModifyGrpclbBalancerChannelArgs(addresses, new_args);
return ModifyGrpclbBalancerChannelArgs(new_args);
}
//
@ -1464,8 +1462,8 @@ void GrpcLb::ProcessAddressesAndChannelArgsLocked(
&args, args_to_remove, GPR_ARRAY_SIZE(args_to_remove), &new_arg, 1);
// Construct args for balancer channel.
ServerAddressList balancer_addresses = ExtractBalancerAddresses(args);
grpc_channel_args* lb_channel_args = BuildBalancerChannelArgs(
balancer_addresses, response_generator_.get(), &args);
grpc_channel_args* lb_channel_args =
BuildBalancerChannelArgs(response_generator_.get(), &args);
// Create balancer channel if needed.
if (lb_channel_ == nullptr) {
std::string uri_str = absl::StrCat("fake:///", server_name_);

@ -24,8 +24,7 @@
namespace grpc_core {
grpc_channel_args* ModifyGrpclbBalancerChannelArgs(
const ServerAddressList& /*addresses*/, grpc_channel_args* args) {
grpc_channel_args* ModifyGrpclbBalancerChannelArgs(grpc_channel_args* args) {
return args;
}

@ -33,8 +33,7 @@ namespace grpc_core {
/// Takes ownership of \a args.
///
/// Caller takes ownership of the returned args.
grpc_channel_args* ModifyGrpclbBalancerChannelArgs(
const ServerAddressList& addresses, grpc_channel_args* args);
grpc_channel_args* ModifyGrpclbBalancerChannelArgs(grpc_channel_args* args);
grpc_channel* CreateGrpclbBalancerChannel(const char* target_uri,
const grpc_channel_args& args);

@ -39,8 +39,7 @@
namespace grpc_core {
grpc_channel_args* ModifyGrpclbBalancerChannelArgs(
const ServerAddressList& /*addresses*/, grpc_channel_args* args) {
grpc_channel_args* ModifyGrpclbBalancerChannelArgs(grpc_channel_args* args) {
absl::InlinedVector<const char*, 1> args_to_remove;
absl::InlinedVector<grpc_arg, 1> args_to_add;
// Substitute the channel credentials with a version without call

@ -93,7 +93,7 @@ class Chttp2ServerListener : public Server::ListenerInterface {
explicit ConfigFetcherWatcher(Chttp2ServerListener* listener)
: listener_(listener) {}
void UpdateConfig(grpc_channel_args* /*args*/) override {
void UpdateConfig(grpc_channel_args* args) override {
{
MutexLock lock(&listener_->mu_);
grpc_channel_args_destroy(listener_->args_);

@ -30,7 +30,7 @@
namespace {
grpc_channel_args* ModifyArgsForConnection(grpc_channel_args* args,
grpc_error** error) {
grpc_error** /*error*/) {
return args;
}

@ -1389,8 +1389,8 @@ grpc_error* LdsResponseParseClient(
grpc_error* LdsResponseParseServer(
upb_arena* arena, const envoy_config_listener_v3_Listener* listener,
const std::string& listener_name,
const envoy_config_core_v3_Address* address,
const std::string& /*listener_name*/,
const envoy_config_core_v3_Address* /*address*/,
XdsApi::LdsUpdate* lds_update) {
lds_update->type = XdsApi::LdsUpdate::ListenerType::kTcpListener;
// TODO(yashykt): Support filter chain match.

@ -59,7 +59,7 @@ class grpc_fake_server_credentials final : public grpc_server_credentials {
~grpc_fake_server_credentials() override = default;
grpc_core::RefCountedPtr<grpc_server_security_connector>
create_security_connector(const grpc_channel_args* args) override {
create_security_connector(const grpc_channel_args* /*args*/) override {
return grpc_fake_server_security_connector_create(this->Ref());
}
};

@ -513,7 +513,7 @@ grpc_resource_user* CreateDefaultResourceUser(const grpc_channel_args* args) {
}
RefCountedPtr<channelz::ServerNode> CreateChannelzNode(
Server* /*server*/, const grpc_channel_args* args) {
const grpc_channel_args* args) {
RefCountedPtr<channelz::ServerNode> channelz_node;
if (grpc_channel_args_find_bool(args, GRPC_ARG_ENABLE_CHANNELZ,
GRPC_ENABLE_CHANNELZ_DEFAULT)) {
@ -534,7 +534,7 @@ RefCountedPtr<channelz::ServerNode> CreateChannelzNode(
Server::Server(const grpc_channel_args* args)
: channel_args_(grpc_channel_args_copy(args)),
default_resource_user_(CreateDefaultResourceUser(args)),
channelz_node_(CreateChannelzNode(this, args)) {}
channelz_node_(CreateChannelzNode(args)) {}
Server::~Server() {
grpc_channel_args_destroy(channel_args_);

@ -28,10 +28,8 @@ using ::opencensus::tags::TagMap;
using ::opencensus::trace::Span;
using ::opencensus::trace::SpanContext;
void GenerateServerContext(absl::string_view tracing,
absl::string_view /*stats*/,
absl::string_view /*primary_role*/,
absl::string_view method, CensusContext* context) {
void GenerateServerContext(absl::string_view tracing, absl::string_view method,
CensusContext* context) {
// Destruct the current CensusContext to free the Span memory before
// overwriting it below.
context->~CensusContext();

@ -96,9 +96,8 @@ size_t ServerStatsDeserialize(const char* buf, size_t buf_size,
// Deserialize the incoming SpanContext and generate a new server context based
// on that. This new span will never be a root span. This should only be called
// with a blank CensusContext as it overwrites it.
void GenerateServerContext(absl::string_view tracing, absl::string_view stats,
absl::string_view primary_role,
absl::string_view method, CensusContext* context);
void GenerateServerContext(absl::string_view tracing, absl::string_view method,
CensusContext* context);
// Creates a new client context that is by default a new root context.
// If the current context is the default context then the newly created

@ -103,19 +103,8 @@ void CensusServerCallData::OnDoneRecvInitialMetadataCb(void* user_data,
size_t tracing_str_len = GRPC_SLICE_IS_EMPTY(sml.tracing_slice)
? 0
: GRPC_SLICE_LENGTH(sml.tracing_slice);
const char* census_str = GRPC_SLICE_IS_EMPTY(sml.census_proto)
? ""
: reinterpret_cast<const char*>(
GRPC_SLICE_START_PTR(sml.census_proto));
size_t census_str_len = GRPC_SLICE_IS_EMPTY(sml.census_proto)
? 0
: GRPC_SLICE_LENGTH(sml.census_proto);
GenerateServerContext(absl::string_view(tracing_str, tracing_str_len),
absl::string_view(census_str, census_str_len),
/*primary_role*/ "", calld->qualified_method_,
&calld->context_);
calld->qualified_method_, &calld->context_);
grpc_slice_unref_internal(sml.tracing_slice);
grpc_slice_unref_internal(sml.census_proto);
grpc_slice_unref_internal(sml.path);

@ -895,8 +895,7 @@ class AdsServiceImpl : public std::enable_shared_from_this<AdsServiceImpl> {
&subscription_state, &resource_state,
update_queue) ||
ClientNeedsResourceUpdate(resource_type_state, resource_state,
client_resource_type_version,
&subscription_state)) {
client_resource_type_version)) {
gpr_log(GPR_INFO, "ADS[%p]: Sending update for type=%s name=%s", this,
request.type_url().c_str(), resource_name.c_str());
resources_added_to_response.emplace(resource_name);
@ -942,11 +941,9 @@ class AdsServiceImpl : public std::enable_shared_from_this<AdsServiceImpl> {
auto& resource_name_map = resource_type_state.resource_name_map;
auto it = subscription_name_map.find(resource_name);
if (it != subscription_name_map.end()) {
SubscriptionState& subscription_state = it->second;
ResourceState& resource_state = resource_name_map[resource_name];
if (ClientNeedsResourceUpdate(resource_type_state, resource_state,
sent_state->resource_type_version,
&subscription_state)) {
sent_state->resource_type_version)) {
gpr_log(GPR_INFO, "ADS[%p]: Sending update for type=%s name=%s", this,
resource_type.c_str(), resource_name.c_str());
response->emplace();
@ -1060,8 +1057,7 @@ class AdsServiceImpl : public std::enable_shared_from_this<AdsServiceImpl> {
// the resource.
static bool ClientNeedsResourceUpdate(
const ResourceTypeState& resource_type_state,
const ResourceState& resource_state, int client_resource_type_version,
SubscriptionState* /*subscription_state*/) {
const ResourceState& resource_state, int client_resource_type_version) {
return client_resource_type_version <
resource_type_state.resource_type_version &&
resource_state.resource_type_version <=

Loading…
Cancel
Save