From 78b2b35ad120efb57b9c039508acb545ac54bcea Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Fri, 24 Feb 2023 11:17:53 -0800 Subject: [PATCH] PollingResolver: clean up the way that ChannelArgs are passed in (#32430) --- .../resolver/dns/c_ares/dns_resolver_ares.cc | 47 ++++++++++--------- .../resolver/dns/native/dns_resolver.cc | 36 +++++++------- .../resolver/polling_resolver.cc | 3 +- .../resolver/polling_resolver.h | 3 +- 4 files changed, 43 insertions(+), 46 deletions(-) diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index 36ab1bc26c3..8277a636a60 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -95,7 +95,7 @@ namespace { class AresClientChannelDNSResolver : public PollingResolver { public: AresClientChannelDNSResolver(ResolverArgs args, - const ChannelArgs& channel_args); + Duration min_time_between_resolutions); OrphanablePtr StartRequest() override; @@ -205,29 +205,26 @@ class AresClientChannelDNSResolver : public PollingResolver { }; AresClientChannelDNSResolver::AresClientChannelDNSResolver( - ResolverArgs args, const ChannelArgs& channel_args) - : PollingResolver( - std::move(args), channel_args, - std::max(Duration::Zero(), - channel_args - .GetDurationFromIntMillis( - GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS) - .value_or(Duration::Seconds(30))), - BackOff::Options() - .set_initial_backoff(Duration::Milliseconds( - GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS * 1000)) - .set_multiplier(GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER) - .set_jitter(GRPC_DNS_RECONNECT_JITTER) - .set_max_backoff(Duration::Milliseconds( - GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000)), - &grpc_trace_cares_resolver), + ResolverArgs args, Duration min_time_between_resolutions) + : PollingResolver(std::move(args), min_time_between_resolutions, + BackOff::Options() + .set_initial_backoff(Duration::Milliseconds( + GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS * 1000)) + .set_multiplier(GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER) + .set_jitter(GRPC_DNS_RECONNECT_JITTER) + .set_max_backoff(Duration::Milliseconds( + GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000)), + &grpc_trace_cares_resolver), request_service_config_( - !channel_args.GetBool(GRPC_ARG_SERVICE_CONFIG_DISABLE_RESOLUTION) + !channel_args() + .GetBool(GRPC_ARG_SERVICE_CONFIG_DISABLE_RESOLUTION) .value_or(true)), - enable_srv_queries_(channel_args.GetBool(GRPC_ARG_DNS_ENABLE_SRV_QUERIES) + enable_srv_queries_(channel_args() + .GetBool(GRPC_ARG_DNS_ENABLE_SRV_QUERIES) .value_or(false)), query_timeout_ms_( - std::max(0, channel_args.GetInt(GRPC_ARG_DNS_ARES_QUERY_TIMEOUT_MS) + std::max(0, channel_args() + .GetInt(GRPC_ARG_DNS_ARES_QUERY_TIMEOUT_MS) .value_or(GRPC_DNS_ARES_DEFAULT_QUERY_TIMEOUT_MS))) {} AresClientChannelDNSResolver::~AresClientChannelDNSResolver() { @@ -460,9 +457,13 @@ class AresClientChannelDNSResolverFactory : public ResolverFactory { } OrphanablePtr CreateResolver(ResolverArgs args) const override { - ChannelArgs channel_args = args.args; - return MakeOrphanable(std::move(args), - channel_args); + Duration min_time_between_resolutions = std::max( + Duration::Zero(), args.args + .GetDurationFromIntMillis( + GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS) + .value_or(Duration::Seconds(30))); + return MakeOrphanable( + std::move(args), min_time_between_resolutions); } }; diff --git a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc index 53b46918efe..06372d844a6 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc @@ -67,7 +67,7 @@ TraceFlag grpc_trace_dns_resolver(false, "dns_resolver"); class NativeClientChannelDNSResolver : public PollingResolver { public: NativeClientChannelDNSResolver(ResolverArgs args, - const ChannelArgs& channel_args); + Duration min_time_between_resolutions); ~NativeClientChannelDNSResolver() override; OrphanablePtr StartRequest() override; @@ -88,22 +88,16 @@ class NativeClientChannelDNSResolver : public PollingResolver { }; NativeClientChannelDNSResolver::NativeClientChannelDNSResolver( - ResolverArgs args, const ChannelArgs& channel_args) - : PollingResolver( - std::move(args), channel_args, - std::max(Duration::Zero(), - channel_args - .GetDurationFromIntMillis( - GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS) - .value_or(Duration::Seconds(30))), - BackOff::Options() - .set_initial_backoff(Duration::Milliseconds( - GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS * 1000)) - .set_multiplier(GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER) - .set_jitter(GRPC_DNS_RECONNECT_JITTER) - .set_max_backoff(Duration::Milliseconds( - GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000)), - &grpc_trace_dns_resolver) { + ResolverArgs args, Duration min_time_between_resolutions) + : PollingResolver(std::move(args), min_time_between_resolutions, + BackOff::Options() + .set_initial_backoff(Duration::Milliseconds( + GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS * 1000)) + .set_multiplier(GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER) + .set_jitter(GRPC_DNS_RECONNECT_JITTER) + .set_max_backoff(Duration::Milliseconds( + GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000)), + &grpc_trace_dns_resolver) { if (GRPC_TRACE_FLAG_ENABLED(grpc_trace_dns_resolver)) { gpr_log(GPR_DEBUG, "[dns_resolver=%p] created", this); } @@ -174,9 +168,13 @@ class NativeClientChannelDNSResolverFactory : public ResolverFactory { OrphanablePtr CreateResolver(ResolverArgs args) const override { if (!IsValidUri(args.uri)) return nullptr; - auto channel_args = args.args; + Duration min_time_between_resolutions = std::max( + Duration::Zero(), args.args + .GetDurationFromIntMillis( + GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS) + .value_or(Duration::Seconds(30))); return MakeOrphanable( - std::move(args), std::move(channel_args)); + std::move(args), min_time_between_resolutions); } }; diff --git a/src/core/ext/filters/client_channel/resolver/polling_resolver.cc b/src/core/ext/filters/client_channel/resolver/polling_resolver.cc index c74a5ef3868..46f63e68e7a 100644 --- a/src/core/ext/filters/client_channel/resolver/polling_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/polling_resolver.cc @@ -47,13 +47,12 @@ namespace grpc_core { using ::grpc_event_engine::experimental::EventEngine; PollingResolver::PollingResolver(ResolverArgs args, - const ChannelArgs& channel_args, Duration min_time_between_resolutions, BackOff::Options backoff_options, TraceFlag* tracer) : authority_(args.uri.authority()), name_to_resolve_(absl::StripPrefix(args.uri.path(), "/")), - channel_args_(channel_args), + channel_args_(std::move(args.args)), work_serializer_(std::move(args.work_serializer)), result_handler_(std::move(args.result_handler)), tracer_(tracer), diff --git a/src/core/ext/filters/client_channel/resolver/polling_resolver.h b/src/core/ext/filters/client_channel/resolver/polling_resolver.h index 1595e0d5c17..c6ba1d6efc6 100644 --- a/src/core/ext/filters/client_channel/resolver/polling_resolver.h +++ b/src/core/ext/filters/client_channel/resolver/polling_resolver.h @@ -44,8 +44,7 @@ namespace grpc_core { // Implementations need only to implement StartRequest(). class PollingResolver : public Resolver { public: - PollingResolver(ResolverArgs args, const ChannelArgs& channel_args, - Duration min_time_between_resolutions, + PollingResolver(ResolverArgs args, Duration min_time_between_resolutions, BackOff::Options backoff_options, TraceFlag* tracer); ~PollingResolver() override;