Code review changes.

pull/8618/head
Mark D. Roth 8 years ago
parent e5b4d4f35b
commit f79ce7d201
  1. 7
      include/grpc/grpc.h
  2. 13
      src/core/ext/client_channel/client_channel.c
  3. 2
      src/core/lib/channel/channel_stack.c
  4. 4
      src/core/lib/channel/channel_stack.h
  5. 2
      src/core/lib/channel/connected_channel.c
  6. 2
      src/core/lib/surface/channel.c
  7. 2
      src/core/lib/surface/lame_client.c
  8. 5
      test/core/client_channel/lb_policies_test.c

@ -237,9 +237,12 @@ GRPCAPI struct census_context *grpc_census_call_get_context(grpc_call *call);
created for. */ created for. */
GRPCAPI char *grpc_channel_get_target(grpc_channel *channel); GRPCAPI char *grpc_channel_get_target(grpc_channel *channel);
/** Request info about the channel. */ /** Request info about the channel.
\a channel_info indicates what information is being requested and
how that information will be returned.
\a channel_info is owned by the caller. */
GRPCAPI void grpc_channel_get_info(grpc_channel *channel, GRPCAPI void grpc_channel_get_info(grpc_channel *channel,
grpc_channel_info *channel_info); const grpc_channel_info *channel_info);
/** Create a client channel to 'target'. Additional channel level configuration /** Create a client channel to 'target'. Additional channel level configuration
MAY be provided by grpc_channel_args, though the expectation is that most MAY be provided by grpc_channel_args, though the expectation is that most

@ -243,7 +243,7 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg,
grpc_channel_args_find(lb_policy_args.args, GRPC_ARG_LB_POLICY_NAME); grpc_channel_args_find(lb_policy_args.args, GRPC_ARG_LB_POLICY_NAME);
if (channel_arg != NULL) { if (channel_arg != NULL) {
GPR_ASSERT(channel_arg->type == GRPC_ARG_STRING); GPR_ASSERT(channel_arg->type == GRPC_ARG_STRING);
lb_policy_name = gpr_strdup(channel_arg->value.string); lb_policy_name = channel_arg->value.string;
} }
// Special case: If all of the addresses are balancer addresses, // Special case: If all of the addresses are balancer addresses,
// assume that we should use the grpclb policy, regardless of what the // assume that we should use the grpclb policy, regardless of what the
@ -267,14 +267,13 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg,
"addresses, no backend addresses -- forcing use of grpclb LB " "addresses, no backend addresses -- forcing use of grpclb LB "
"policy", "policy",
lb_policy_name); lb_policy_name);
gpr_free(lb_policy_name);
} }
lb_policy_name = gpr_strdup("grpclb"); lb_policy_name = "grpclb";
} }
} }
// Use pick_first if nothing was specified and we didn't select grpclb // Use pick_first if nothing was specified and we didn't select grpclb
// above. // above.
if (lb_policy_name == NULL) lb_policy_name = gpr_strdup("pick_first"); if (lb_policy_name == NULL) lb_policy_name = "pick_first";
lb_policy = lb_policy =
grpc_lb_policy_create(exec_ctx, lb_policy_name, &lb_policy_args); grpc_lb_policy_create(exec_ctx, lb_policy_name, &lb_policy_args);
@ -292,6 +291,10 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg,
(grpc_method_config_table *)channel_arg->value.pointer.p, (grpc_method_config_table *)channel_arg->value.pointer.p,
method_config_convert_value, &method_parameters_vtable); method_config_convert_value, &method_parameters_vtable);
} }
// Before we clean up, save a copy of lb_policy_name, since it might
// be pointing to data inside chand->resolver_result.
// The copy will be saved in chand->lb_policy_name below.
lb_policy_name = gpr_strdup(lb_policy_name);
grpc_channel_args_destroy(chand->resolver_result); grpc_channel_args_destroy(chand->resolver_result);
chand->resolver_result = NULL; chand->resolver_result = NULL;
} }
@ -435,7 +438,7 @@ static void cc_start_transport_op(grpc_exec_ctx *exec_ctx,
static void cc_get_channel_info(grpc_exec_ctx *exec_ctx, static void cc_get_channel_info(grpc_exec_ctx *exec_ctx,
grpc_channel_element *elem, grpc_channel_element *elem,
grpc_channel_info *info) { const grpc_channel_info *info) {
channel_data *chand = elem->channel_data; channel_data *chand = elem->channel_data;
gpr_mu_lock(&chand->mu); gpr_mu_lock(&chand->mu);
if (info->lb_policy_name != NULL) { if (info->lb_policy_name != NULL) {

@ -257,7 +257,7 @@ char *grpc_call_next_get_peer(grpc_exec_ctx *exec_ctx,
void grpc_channel_next_get_info(grpc_exec_ctx *exec_ctx, void grpc_channel_next_get_info(grpc_exec_ctx *exec_ctx,
grpc_channel_element *elem, grpc_channel_element *elem,
grpc_channel_info *channel_info) { const grpc_channel_info *channel_info) {
grpc_channel_element *next_elem = elem + 1; grpc_channel_element *next_elem = elem + 1;
return next_elem->filter->get_channel_info(exec_ctx, next_elem, channel_info); return next_elem->filter->get_channel_info(exec_ctx, next_elem, channel_info);
} }

@ -158,7 +158,7 @@ typedef struct {
/* Implement grpc_channel_get_info() */ /* Implement grpc_channel_get_info() */
void (*get_channel_info)(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem, void (*get_channel_info)(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem,
grpc_channel_info *channel_info); const grpc_channel_info *channel_info);
/* The name of this filter */ /* The name of this filter */
const char *name; const char *name;
@ -280,7 +280,7 @@ char *grpc_call_next_get_peer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem);
/* Pass through a request to get_channel_info() to the next child element */ /* Pass through a request to get_channel_info() to the next child element */
void grpc_channel_next_get_info(grpc_exec_ctx *exec_ctx, void grpc_channel_next_get_info(grpc_exec_ctx *exec_ctx,
grpc_channel_element *elem, grpc_channel_element *elem,
grpc_channel_info *channel_info); const grpc_channel_info *channel_info);
/* Given the top element of a channel stack, get the channel stack itself */ /* Given the top element of a channel stack, get the channel stack itself */
grpc_channel_stack *grpc_channel_stack_from_top_element( grpc_channel_stack *grpc_channel_stack_from_top_element(

@ -137,7 +137,7 @@ static char *con_get_peer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem) {
/* No-op. */ /* No-op. */
static void con_get_channel_info(grpc_exec_ctx *exec_ctx, static void con_get_channel_info(grpc_exec_ctx *exec_ctx,
grpc_channel_element *elem, grpc_channel_element *elem,
grpc_channel_info *channel_info) {} const grpc_channel_info *channel_info) {}
static const grpc_channel_filter connected_channel_filter = { static const grpc_channel_filter connected_channel_filter = {
con_start_transport_stream_op, con_start_transport_stream_op,

@ -176,7 +176,7 @@ char *grpc_channel_get_target(grpc_channel *channel) {
} }
void grpc_channel_get_info(grpc_channel *channel, void grpc_channel_get_info(grpc_channel *channel,
grpc_channel_info *channel_info) { const grpc_channel_info *channel_info) {
grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
grpc_channel_element *elem = grpc_channel_element *elem =
grpc_channel_stack_element(CHANNEL_STACK_FROM_CHANNEL(channel), 0); grpc_channel_stack_element(CHANNEL_STACK_FROM_CHANNEL(channel), 0);

@ -90,7 +90,7 @@ static char *lame_get_peer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem) {
static void lame_get_channel_info(grpc_exec_ctx *exec_ctx, static void lame_get_channel_info(grpc_exec_ctx *exec_ctx,
grpc_channel_element *elem, grpc_channel_element *elem,
grpc_channel_info *channel_info) {} const grpc_channel_info *channel_info) {}
static void lame_start_transport_op(grpc_exec_ctx *exec_ctx, static void lame_start_transport_op(grpc_exec_ctx *exec_ctx,
grpc_channel_element *elem, grpc_channel_element *elem,

@ -643,6 +643,7 @@ static void test_get_channel_info() {
"test:127.0.0.1:1234?lb_policy=round_robin", NULL, NULL); "test:127.0.0.1:1234?lb_policy=round_robin", NULL, NULL);
// Ensures that resolver returns. // Ensures that resolver returns.
grpc_channel_check_connectivity_state(channel, true /* try_to_connect */); grpc_channel_check_connectivity_state(channel, true /* try_to_connect */);
// Use grpc_channel_get_info() to get LB policy name.
char *lb_policy_name = NULL; char *lb_policy_name = NULL;
grpc_channel_info channel_info; grpc_channel_info channel_info;
channel_info.lb_policy_name = &lb_policy_name; channel_info.lb_policy_name = &lb_policy_name;
@ -650,6 +651,10 @@ static void test_get_channel_info() {
GPR_ASSERT(lb_policy_name != NULL); GPR_ASSERT(lb_policy_name != NULL);
GPR_ASSERT(strcmp(lb_policy_name, "round_robin") == 0); GPR_ASSERT(strcmp(lb_policy_name, "round_robin") == 0);
gpr_free(lb_policy_name); gpr_free(lb_policy_name);
// Try again without requesting anything. This is a no-op.
channel_info.lb_policy_name = NULL;
grpc_channel_get_info(channel, &channel_info);
// Clean up.
grpc_channel_destroy(channel); grpc_channel_destroy(channel);
} }

Loading…
Cancel
Save