From 1621c26c1ec49207e994e4b11671fb8fb38225ba Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 15 Sep 2016 12:51:29 -0700 Subject: [PATCH] Allow resolver to pass channel args to LB policy. --- src/core/ext/client_config/client_channel.c | 2 ++ .../ext/client_config/lb_policy_factory.h | 3 +++ src/core/ext/client_config/resolver_result.c | 15 +++++++++++-- src/core/ext/client_config/resolver_result.h | 21 ++++++++++++++++--- .../ext/resolver/dns/native/dns_resolver.c | 2 +- .../ext/resolver/sockaddr/sockaddr_resolver.c | 2 +- 6 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index 2e6f253d386..0343a5e2544 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -179,6 +179,8 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg, grpc_lb_policy_args lb_policy_args; lb_policy_args.addresses = grpc_resolver_result_get_addresses(chand->resolver_result); + lb_policy_args.additional_args = + grpc_resolver_result_get_lb_policy_args(chand->resolver_result); lb_policy_args.client_channel_factory = chand->client_channel_factory; lb_policy = grpc_lb_policy_create( exec_ctx, diff --git a/src/core/ext/client_config/lb_policy_factory.h b/src/core/ext/client_config/lb_policy_factory.h index 6919b1eb984..e2364c31a82 100644 --- a/src/core/ext/client_config/lb_policy_factory.h +++ b/src/core/ext/client_config/lb_policy_factory.h @@ -47,9 +47,12 @@ struct grpc_lb_policy_factory { const grpc_lb_policy_factory_vtable *vtable; }; +// TODO(roth, ctiller): Consider replacing this struct with +// grpc_channel_args. See comment in resolver_result.h for details. typedef struct grpc_lb_policy_args { grpc_addresses *addresses; grpc_client_channel_factory *client_channel_factory; + grpc_channel_args *additional_args; } grpc_lb_policy_args; struct grpc_lb_policy_factory_vtable { diff --git a/src/core/ext/client_config/resolver_result.c b/src/core/ext/client_config/resolver_result.c index ac263b9a468..f97dc467e1d 100644 --- a/src/core/ext/client_config/resolver_result.c +++ b/src/core/ext/client_config/resolver_result.c @@ -36,6 +36,8 @@ #include #include +#include "src/core/lib/channel/channel_args.h" + grpc_addresses* grpc_addresses_create(size_t num_addresses) { grpc_addresses* addresses = gpr_malloc(sizeof(grpc_addresses)); addresses->num_addresses = num_addresses; @@ -71,15 +73,18 @@ struct grpc_resolver_result { gpr_refcount refs; grpc_addresses* addresses; char* lb_policy_name; + grpc_channel_args* lb_policy_args; }; -grpc_resolver_result* grpc_resolver_result_create(grpc_addresses* addresses, - const char* lb_policy_name) { +grpc_resolver_result* grpc_resolver_result_create( + grpc_addresses* addresses, const char* lb_policy_name, + grpc_channel_args* lb_policy_args) { grpc_resolver_result* result = gpr_malloc(sizeof(*result)); memset(result, 0, sizeof(*result)); gpr_ref_init(&result->refs, 1); result->addresses = addresses; result->lb_policy_name = gpr_strdup(lb_policy_name); + result->lb_policy_args = lb_policy_args; return result; } @@ -92,6 +97,7 @@ void grpc_resolver_result_unref(grpc_exec_ctx* exec_ctx, if (gpr_unref(&result->refs)) { grpc_addresses_destroy(result->addresses); gpr_free(result->lb_policy_name); + grpc_channel_args_destroy(result->lb_policy_args); gpr_free(result); } } @@ -105,3 +111,8 @@ const char* grpc_resolver_result_get_lb_policy_name( grpc_resolver_result* result) { return result->lb_policy_name; } + +grpc_channel_args* grpc_resolver_result_get_lb_policy_args( + grpc_resolver_result* result) { + return result->lb_policy_args; +} diff --git a/src/core/ext/client_config/resolver_result.h b/src/core/ext/client_config/resolver_result.h index fa60c8cc6d4..821208f7099 100644 --- a/src/core/ext/client_config/resolver_result.h +++ b/src/core/ext/client_config/resolver_result.h @@ -37,6 +37,16 @@ #include "src/core/ext/client_config/lb_policy.h" #include "src/core/lib/iomgr/resolve_address.h" +// TODO(roth, ctiller): In the long term, we are considering replacing +// the resolver_result data structure with grpc_channel_args. The idea is +// that the resolver will return a set of channel args that contains the +// information that is currently in the resolver_result struct. For +// example, there will be specific args indicating the set of addresses +// and the name of the LB policy to instantiate. Note that if we did +// this, we would probably want to change the data structure of +// grpc_channel_args such to a hash table or AVL or some other data +// structure that does not require linear search to find keys. + /// Used to represent addresses returned by the resolver. typedef struct grpc_address { grpc_resolved_address address; @@ -63,9 +73,10 @@ void grpc_addresses_destroy(grpc_addresses* addresses); /// Results reported from a grpc_resolver. typedef struct grpc_resolver_result grpc_resolver_result; -/// Takes ownership of \a addresses. -grpc_resolver_result* grpc_resolver_result_create(grpc_addresses* addresses, - const char* lb_policy_name); +/// Takes ownership of \a addresses and \a lb_policy_args. +grpc_resolver_result* grpc_resolver_result_create( + grpc_addresses* addresses, const char* lb_policy_name, + grpc_channel_args* lb_policy_args); void grpc_resolver_result_ref(grpc_resolver_result* result); void grpc_resolver_result_unref(grpc_exec_ctx* exec_ctx, grpc_resolver_result* result); @@ -78,4 +89,8 @@ grpc_addresses* grpc_resolver_result_get_addresses( const char* grpc_resolver_result_get_lb_policy_name( grpc_resolver_result* result); +/// Caller does NOT take ownership of result. +grpc_channel_args* grpc_resolver_result_get_lb_policy_args( + grpc_resolver_result* result); + #endif /* GRPC_CORE_EXT_CLIENT_CONFIG_RESOLVER_RESULT_H */ diff --git a/src/core/ext/resolver/dns/native/dns_resolver.c b/src/core/ext/resolver/dns/native/dns_resolver.c index 78a996788dc..103d3effbd0 100644 --- a/src/core/ext/resolver/dns/native/dns_resolver.c +++ b/src/core/ext/resolver/dns/native/dns_resolver.c @@ -174,7 +174,7 @@ static void dns_on_resolved(grpc_exec_ctx *exec_ctx, void *arg, false /* is_balancer */); } grpc_resolved_addresses_destroy(r->addresses); - result = grpc_resolver_result_create(addresses, r->lb_policy_name); + result = grpc_resolver_result_create(addresses, r->lb_policy_name, NULL); } else { gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); gpr_timespec next_try = gpr_backoff_step(&r->backoff_state, now); diff --git a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c index 5c59e4397af..fb67f1a227f 100644 --- a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c +++ b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c @@ -121,7 +121,7 @@ static void sockaddr_maybe_finish_next_locked(grpc_exec_ctx *exec_ctx, if (r->next_completion != NULL && !r->published) { r->published = true; *r->target_result = grpc_resolver_result_create( - grpc_addresses_copy(r->addresses), r->lb_policy_name); + grpc_addresses_copy(r->addresses), r->lb_policy_name, NULL); grpc_exec_ctx_sched(exec_ctx, r->next_completion, GRPC_ERROR_NONE, NULL); r->next_completion = NULL; }