diff options
| -rw-r--r-- | CMakeLists.txt | 37 | ||||
| -rw-r--r-- | Makefile | 37 | ||||
| -rw-r--r-- | build.yaml | 12 | ||||
| -rw-r--r-- | include/grpc/impl/codegen/grpc_types.h | 3 | ||||
| -rw-r--r-- | src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc | 84 | ||||
| -rw-r--r-- | src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc | 89 | ||||
| -rw-r--r-- | test/core/client_channel/resolvers/BUILD | 12 | ||||
| -rw-r--r-- | test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc | 304 | ||||
| -rw-r--r-- | tools/run_tests/generated/sources_and_headers.json | 17 | ||||
| -rw-r--r-- | tools/run_tests/generated/tests.json | 24 |
10 files changed, 561 insertions, 58 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt index 906be66173..6ec3e803e7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -227,6 +227,7 @@ add_dependencies(buildtests_c compression_test) add_dependencies(buildtests_c concurrent_connectivity_test) add_dependencies(buildtests_c connection_refused_test) add_dependencies(buildtests_c dns_resolver_connectivity_test) +add_dependencies(buildtests_c dns_resolver_cooldown_test) add_dependencies(buildtests_c dns_resolver_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_c dualstack_socket_test) @@ -2649,15 +2650,6 @@ target_link_libraries(grpc++_core_stats grpc++ ) -foreach(_hdr - src/cpp/util/core_stats.h -) - string(REPLACE "include/" "" _path ${_hdr}) - get_filename_component(_path ${_path} PATH) - install(FILES ${_hdr} - DESTINATION "${gRPC_INSTALL_INCLUDEDIR}/${_path}" - ) -endforeach() endif (gRPC_BUILD_TESTS) @@ -5259,6 +5251,33 @@ target_link_libraries(dns_resolver_connectivity_test endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) +add_executable(dns_resolver_cooldown_test + test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc +) + + +target_include_directories(dns_resolver_cooldown_test + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include + PRIVATE ${_gRPC_SSL_INCLUDE_DIR} + PRIVATE ${_gRPC_PROTOBUF_INCLUDE_DIR} + PRIVATE ${_gRPC_ZLIB_INCLUDE_DIR} + PRIVATE ${_gRPC_BENCHMARK_INCLUDE_DIR} + PRIVATE ${_gRPC_CARES_INCLUDE_DIR} + PRIVATE ${_gRPC_GFLAGS_INCLUDE_DIR} +) + +target_link_libraries(dns_resolver_cooldown_test + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc_test_util + grpc + gpr_test_util + gpr +) + +endif (gRPC_BUILD_TESTS) +if (gRPC_BUILD_TESTS) + add_executable(dns_resolver_test test/core/client_channel/resolvers/dns_resolver_test.cc ) @@ -971,6 +971,7 @@ compression_test: $(BINDIR)/$(CONFIG)/compression_test concurrent_connectivity_test: $(BINDIR)/$(CONFIG)/concurrent_connectivity_test connection_refused_test: $(BINDIR)/$(CONFIG)/connection_refused_test dns_resolver_connectivity_test: $(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test +dns_resolver_cooldown_test: $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_test dns_resolver_test: $(BINDIR)/$(CONFIG)/dns_resolver_test dualstack_socket_test: $(BINDIR)/$(CONFIG)/dualstack_socket_test endpoint_pair_test: $(BINDIR)/$(CONFIG)/endpoint_pair_test @@ -1386,6 +1387,7 @@ buildtests_c: privatelibs_c \ $(BINDIR)/$(CONFIG)/concurrent_connectivity_test \ $(BINDIR)/$(CONFIG)/connection_refused_test \ $(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test \ + $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_test \ $(BINDIR)/$(CONFIG)/dns_resolver_test \ $(BINDIR)/$(CONFIG)/dualstack_socket_test \ $(BINDIR)/$(CONFIG)/endpoint_pair_test \ @@ -1840,6 +1842,8 @@ test_c: buildtests_c $(Q) $(BINDIR)/$(CONFIG)/connection_refused_test || ( echo test connection_refused_test failed ; exit 1 ) $(E) "[RUN] Testing dns_resolver_connectivity_test" $(Q) $(BINDIR)/$(CONFIG)/dns_resolver_connectivity_test || ( echo test dns_resolver_connectivity_test failed ; exit 1 ) + $(E) "[RUN] Testing dns_resolver_cooldown_test" + $(Q) $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_test || ( echo test dns_resolver_cooldown_test failed ; exit 1 ) $(E) "[RUN] Testing dns_resolver_test" $(Q) $(BINDIR)/$(CONFIG)/dns_resolver_test || ( echo test dns_resolver_test failed ; exit 1 ) $(E) "[RUN] Testing dualstack_socket_test" @@ -4810,7 +4814,6 @@ LIBGRPC++_CORE_STATS_SRC = \ src/cpp/util/core_stats.cc \ PUBLIC_HEADERS_CXX += \ - src/cpp/util/core_stats.h \ LIBGRPC++_CORE_STATS_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(LIBGRPC++_CORE_STATS_SRC)))) @@ -9928,6 +9931,38 @@ endif endif +DNS_RESOLVER_COOLDOWN_TEST_SRC = \ + test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc \ + +DNS_RESOLVER_COOLDOWN_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(DNS_RESOLVER_COOLDOWN_TEST_SRC)))) +ifeq ($(NO_SECURE),true) + +# You can't build secure targets if you don't have OpenSSL. + +$(BINDIR)/$(CONFIG)/dns_resolver_cooldown_test: openssl_dep_error + +else + + + +$(BINDIR)/$(CONFIG)/dns_resolver_cooldown_test: $(DNS_RESOLVER_COOLDOWN_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + $(E) "[LD] Linking $@" + $(Q) mkdir -p `dirname $@` + $(Q) $(LD) $(LDFLAGS) $(DNS_RESOLVER_COOLDOWN_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_test + +endif + +$(OBJDIR)/$(CONFIG)/test/core/client_channel/resolvers/dns_resolver_cooldown_test.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + +deps_dns_resolver_cooldown_test: $(DNS_RESOLVER_COOLDOWN_TEST_OBJS:.o=.dep) + +ifneq ($(NO_SECURE),true) +ifneq ($(NO_DEPS),true) +-include $(DNS_RESOLVER_COOLDOWN_TEST_OBJS:.o=.dep) +endif +endif + + DNS_RESOLVER_TEST_SRC = \ test/core/client_channel/resolvers/dns_resolver_test.cc \ diff --git a/build.yaml b/build.yaml index 76fa639c7a..08ecff013a 100644 --- a/build.yaml +++ b/build.yaml @@ -1336,7 +1336,7 @@ libs: - name: grpc++_core_stats build: private language: c++ - public_headers: + headers: - src/cpp/util/core_stats.h src: - src/proto/grpc/core/stats.proto @@ -1940,6 +1940,16 @@ targets: - gpr exclude_iomgrs: - uv +- name: dns_resolver_cooldown_test + build: test + language: c + src: + - test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc + deps: + - grpc_test_util + - grpc + - gpr_test_util + - gpr - name: dns_resolver_test build: test language: c diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index e5aa29b88a..d64b23f332 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -239,6 +239,9 @@ typedef struct { /** The time between the first and second connection attempts, in ms */ #define GRPC_ARG_INITIAL_RECONNECT_BACKOFF_MS \ "grpc.initial_reconnect_backoff_ms" +/** Minimum amount of time between DNS resolutions, in ms */ +#define GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS \ + "grpc.dns_min_time_between_resolutions_ms" /** The timeout used on servers for finishing handshaking on an incoming connection. Defaults to 120 seconds. */ #define GRPC_ARG_SERVER_HANDSHAKE_TIMEOUT_MS "grpc.server_handshake_timeout_ms" 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 0315a29d15..e5b2815af8 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 @@ -66,8 +66,8 @@ typedef struct { grpc_pollset_set* interested_parties; /** Closures used by the combiner */ - grpc_closure dns_ares_on_retry_timer_locked; - grpc_closure dns_ares_on_resolved_locked; + grpc_closure dns_ares_on_next_resolution_timer_closure; + grpc_closure dns_ares_on_resolved_closure; /** Combiner guarding the rest of the state */ grpc_combiner* combiner; @@ -85,12 +85,15 @@ typedef struct { grpc_channel_args** target_result; /** current (fully resolved) result */ grpc_channel_args* resolved_result; - /** retry timer */ - bool have_retry_timer; - grpc_timer retry_timer; + /** next resolution timer */ + bool have_next_resolution_timer; + grpc_timer next_resolution_timer; /** retry backoff state */ grpc_core::ManualConstructor<grpc_core::BackOff> backoff; - + /** min resolution period. Max one resolution will happen per period */ + grpc_millis min_time_between_resolutions; + /** when was the last resolution? -1 if no resolution has happened yet */ + grpc_millis last_resolution_timestamp; /** currently resolving addresses */ grpc_lb_addresses* lb_addresses; /** currently resolving service config */ @@ -100,6 +103,7 @@ typedef struct { static void dns_ares_destroy(grpc_resolver* r); static void dns_ares_start_resolving_locked(ares_dns_resolver* r); +static void dns_ares_maybe_start_resolving_locked(ares_dns_resolver* r); static void dns_ares_maybe_finish_next_locked(ares_dns_resolver* r); static void dns_ares_shutdown_locked(grpc_resolver* r); @@ -114,8 +118,8 @@ static const grpc_resolver_vtable dns_ares_resolver_vtable = { static void dns_ares_shutdown_locked(grpc_resolver* resolver) { ares_dns_resolver* r = (ares_dns_resolver*)resolver; - if (r->have_retry_timer) { - grpc_timer_cancel(&r->retry_timer); + if (r->have_next_resolution_timer) { + grpc_timer_cancel(&r->next_resolution_timer); } if (r->pending_request != nullptr) { grpc_cancel_ares_request(r->pending_request); @@ -131,19 +135,20 @@ static void dns_ares_shutdown_locked(grpc_resolver* resolver) { static void dns_ares_channel_saw_error_locked(grpc_resolver* resolver) { ares_dns_resolver* r = (ares_dns_resolver*)resolver; if (!r->resolving) { - dns_ares_start_resolving_locked(r); + dns_ares_maybe_start_resolving_locked(r); } } -static void dns_ares_on_retry_timer_locked(void* arg, grpc_error* error) { +static void dns_ares_on_next_resolution_timer_locked(void* arg, + grpc_error* error) { ares_dns_resolver* r = (ares_dns_resolver*)arg; - r->have_retry_timer = false; + r->have_next_resolution_timer = false; if (error == GRPC_ERROR_NONE) { if (!r->resolving) { dns_ares_start_resolving_locked(r); } } - GRPC_RESOLVER_UNREF(&r->base, "retry-timer"); + GRPC_RESOLVER_UNREF(&r->base, "next_resolution_timer"); } static bool value_in_json_array(grpc_json* array, const char* value) { @@ -270,21 +275,22 @@ static void dns_ares_on_resolved_locked(void* arg, grpc_error* error) { grpc_millis timeout = next_try - grpc_core::ExecCtx::Get()->Now(); gpr_log(GPR_INFO, "dns resolution failed (will retry): %s", grpc_error_string(error)); - GPR_ASSERT(!r->have_retry_timer); - r->have_retry_timer = true; - GRPC_RESOLVER_REF(&r->base, "retry-timer"); + GPR_ASSERT(!r->have_next_resolution_timer); + r->have_next_resolution_timer = true; + GRPC_RESOLVER_REF(&r->base, "next_resolution_timer"); if (timeout > 0) { gpr_log(GPR_DEBUG, "retrying in %" PRIdPTR " milliseconds", timeout); } else { gpr_log(GPR_DEBUG, "retrying immediately"); } - grpc_timer_init(&r->retry_timer, next_try, - &r->dns_ares_on_retry_timer_locked); + grpc_timer_init(&r->next_resolution_timer, next_try, + &r->dns_ares_on_next_resolution_timer_closure); } if (r->resolved_result != nullptr) { grpc_channel_args_destroy(r->resolved_result); } r->resolved_result = result; + r->last_resolution_timestamp = grpc_core::ExecCtx::Get()->Now(); r->resolved_version++; dns_ares_maybe_finish_next_locked(r); GRPC_RESOLVER_UNREF(&r->base, "dns-resolving"); @@ -299,7 +305,7 @@ static void dns_ares_next_locked(grpc_resolver* resolver, r->next_completion = on_complete; r->target_result = target_result; if (r->resolved_version == 0 && !r->resolving) { - dns_ares_start_resolving_locked(r); + dns_ares_maybe_start_resolving_locked(r); } else { dns_ares_maybe_finish_next_locked(r); } @@ -313,7 +319,7 @@ static void dns_ares_start_resolving_locked(ares_dns_resolver* r) { r->service_config_json = nullptr; r->pending_request = grpc_dns_lookup_ares( r->dns_server, r->name_to_resolve, r->default_port, r->interested_parties, - &r->dns_ares_on_resolved_locked, &r->lb_addresses, + &r->dns_ares_on_resolved_closure, &r->lb_addresses, true /* check_grpclb */, r->request_service_config ? &r->service_config_json : nullptr); } @@ -331,6 +337,35 @@ static void dns_ares_maybe_finish_next_locked(ares_dns_resolver* r) { } } +static void dns_ares_maybe_start_resolving_locked(ares_dns_resolver* r) { + if (r->last_resolution_timestamp >= 0) { + const grpc_millis earliest_next_resolution = + r->last_resolution_timestamp + r->min_time_between_resolutions; + const grpc_millis ms_until_next_resolution = + earliest_next_resolution - grpc_core::ExecCtx::Get()->Now(); + if (ms_until_next_resolution > 0) { + const grpc_millis last_resolution_ago = + grpc_core::ExecCtx::Get()->Now() - r->last_resolution_timestamp; + gpr_log(GPR_DEBUG, + "In cooldown from last resolution (from %" PRIdPTR + " ms ago). Will resolve again in %" PRIdPTR " ms", + last_resolution_ago, ms_until_next_resolution); + if (!r->have_next_resolution_timer) { + r->have_next_resolution_timer = true; + GRPC_RESOLVER_REF(&r->base, "next_resolution_timer_cooldown"); + grpc_timer_init(&r->next_resolution_timer, ms_until_next_resolution, + &r->dns_ares_on_next_resolution_timer_closure); + } + // TODO(dgq): remove the following two lines once Pick First stops + // discarding subchannels after selecting. + ++r->resolved_version; + dns_ares_maybe_finish_next_locked(r); + return; + } + } + dns_ares_start_resolving_locked(r); +} + static void dns_ares_destroy(grpc_resolver* gr) { gpr_log(GPR_DEBUG, "dns_ares_destroy"); ares_dns_resolver* r = (ares_dns_resolver*)gr; @@ -375,12 +410,17 @@ static grpc_resolver* dns_ares_create(grpc_resolver_args* args, .set_jitter(GRPC_DNS_RECONNECT_JITTER) .set_max_backoff(GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000); r->backoff.Init(grpc_core::BackOff(backoff_options)); - GRPC_CLOSURE_INIT(&r->dns_ares_on_retry_timer_locked, - dns_ares_on_retry_timer_locked, r, + GRPC_CLOSURE_INIT(&r->dns_ares_on_next_resolution_timer_closure, + dns_ares_on_next_resolution_timer_locked, r, grpc_combiner_scheduler(r->base.combiner)); - GRPC_CLOSURE_INIT(&r->dns_ares_on_resolved_locked, + GRPC_CLOSURE_INIT(&r->dns_ares_on_resolved_closure, dns_ares_on_resolved_locked, r, grpc_combiner_scheduler(r->base.combiner)); + const grpc_arg* period_arg = grpc_channel_args_find( + args->args, GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS); + r->min_time_between_resolutions = + grpc_channel_arg_get_integer(period_arg, {1000, 0, INT_MAX}); + r->last_resolution_timestamp = -1; return &r->base; } 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 d071c3ca42..2c798188d1 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 @@ -19,11 +19,13 @@ #include <grpc/support/port_platform.h> #include <inttypes.h> -#include <string.h> +#include <climits> +#include <cstring> #include <grpc/support/alloc.h> #include <grpc/support/host_port.h> #include <grpc/support/string_util.h> +#include <grpc/support/time.h> #include "src/core/ext/filters/client_channel/lb_policy_registry.h" #include "src/core/ext/filters/client_channel/resolver_registry.h" @@ -65,13 +67,16 @@ typedef struct { grpc_channel_args** target_result; /** current (fully resolved) result */ grpc_channel_args* resolved_result; - /** retry timer */ - bool have_retry_timer; - grpc_timer retry_timer; - grpc_closure on_retry; + /** next resolution timer */ + bool have_next_resolution_timer; + grpc_timer next_resolution_timer; + grpc_closure next_resolution_closure; /** retry backoff state */ grpc_core::ManualConstructor<grpc_core::BackOff> backoff; - + /** min resolution period. Max one resolution will happen per period */ + grpc_millis min_time_between_resolutions; + /** when was the last resolution? -1 if no resolution has happened yet */ + grpc_millis last_resolution_timestamp; /** currently resolving addresses */ grpc_resolved_addresses* addresses; } dns_resolver; @@ -79,6 +84,7 @@ typedef struct { static void dns_destroy(grpc_resolver* r); static void dns_start_resolving_locked(dns_resolver* r); +static void maybe_start_resolving_locked(dns_resolver* r); static void dns_maybe_finish_next_locked(dns_resolver* r); static void dns_shutdown_locked(grpc_resolver* r); @@ -92,8 +98,8 @@ static const grpc_resolver_vtable dns_resolver_vtable = { static void dns_shutdown_locked(grpc_resolver* resolver) { dns_resolver* r = (dns_resolver*)resolver; - if (r->have_retry_timer) { - grpc_timer_cancel(&r->retry_timer); + if (r->have_next_resolution_timer) { + grpc_timer_cancel(&r->next_resolution_timer); } if (r->next_completion != nullptr) { *r->target_result = nullptr; @@ -106,7 +112,7 @@ static void dns_shutdown_locked(grpc_resolver* resolver) { static void dns_channel_saw_error_locked(grpc_resolver* resolver) { dns_resolver* r = (dns_resolver*)resolver; if (!r->resolving) { - dns_start_resolving_locked(r); + maybe_start_resolving_locked(r); } } @@ -118,23 +124,19 @@ static void dns_next_locked(grpc_resolver* resolver, r->next_completion = on_complete; r->target_result = target_result; if (r->resolved_version == 0 && !r->resolving) { - dns_start_resolving_locked(r); + maybe_start_resolving_locked(r); } else { dns_maybe_finish_next_locked(r); } } -static void dns_on_retry_timer_locked(void* arg, grpc_error* error) { +static void dns_on_next_resolution_timer_locked(void* arg, grpc_error* error) { dns_resolver* r = (dns_resolver*)arg; - - r->have_retry_timer = false; - if (error == GRPC_ERROR_NONE) { - if (!r->resolving) { - dns_start_resolving_locked(r); - } + r->have_next_resolution_timer = false; + if (error == GRPC_ERROR_NONE && !r->resolving) { + dns_start_resolving_locked(r); } - - GRPC_RESOLVER_UNREF(&r->base, "retry-timer"); + GRPC_RESOLVER_UNREF(&r->base, "next_resolution_timer"); } static void dns_on_resolved_locked(void* arg, grpc_error* error) { @@ -166,17 +168,16 @@ static void dns_on_resolved_locked(void* arg, grpc_error* error) { grpc_millis timeout = next_try - grpc_core::ExecCtx::Get()->Now(); gpr_log(GPR_INFO, "dns resolution failed (will retry): %s", grpc_error_string(error)); - GPR_ASSERT(!r->have_retry_timer); - r->have_retry_timer = true; - GRPC_RESOLVER_REF(&r->base, "retry-timer"); + GPR_ASSERT(!r->have_next_resolution_timer); + r->have_next_resolution_timer = true; + GRPC_RESOLVER_REF(&r->base, "next_resolution_timer"); if (timeout > 0) { gpr_log(GPR_DEBUG, "retrying in %" PRIdPTR " milliseconds", timeout); } else { gpr_log(GPR_DEBUG, "retrying immediately"); } - GRPC_CLOSURE_INIT(&r->on_retry, dns_on_retry_timer_locked, r, - grpc_combiner_scheduler(r->base.combiner)); - grpc_timer_init(&r->retry_timer, next_try, &r->on_retry); + grpc_timer_init(&r->next_resolution_timer, next_try, + &r->next_resolution_closure); } if (r->resolved_result != nullptr) { grpc_channel_args_destroy(r->resolved_result); @@ -189,6 +190,35 @@ static void dns_on_resolved_locked(void* arg, grpc_error* error) { GRPC_RESOLVER_UNREF(&r->base, "dns-resolving"); } +static void maybe_start_resolving_locked(dns_resolver* r) { + if (r->last_resolution_timestamp >= 0) { + const grpc_millis earliest_next_resolution = + r->last_resolution_timestamp + r->min_time_between_resolutions; + const grpc_millis ms_until_next_resolution = + earliest_next_resolution - grpc_core::ExecCtx::Get()->Now(); + if (ms_until_next_resolution > 0) { + const grpc_millis last_resolution_ago = + grpc_core::ExecCtx::Get()->Now() - r->last_resolution_timestamp; + gpr_log(GPR_DEBUG, + "In cooldown from last resolution (from %" PRIdPTR + " ms ago). Will resolve again in %" PRIdPTR " ms", + last_resolution_ago, ms_until_next_resolution); + if (!r->have_next_resolution_timer) { + r->have_next_resolution_timer = true; + GRPC_RESOLVER_REF(&r->base, "next_resolution_timer_cooldown"); + grpc_timer_init(&r->next_resolution_timer, ms_until_next_resolution, + &r->next_resolution_closure); + } + // TODO(dgq): remove the following two lines once Pick First stops + // discarding subchannels after selecting. + ++r->resolved_version; + dns_maybe_finish_next_locked(r); + return; + } + } + dns_start_resolving_locked(r); +} + static void dns_start_resolving_locked(dns_resolver* r) { GRPC_RESOLVER_REF(&r->base, "dns-resolving"); GPR_ASSERT(!r->resolving); @@ -199,6 +229,7 @@ static void dns_start_resolving_locked(dns_resolver* r) { GRPC_CLOSURE_CREATE(dns_on_resolved_locked, r, grpc_combiner_scheduler(r->base.combiner)), &r->addresses); + r->last_resolution_timestamp = grpc_core::ExecCtx::Get()->Now(); } static void dns_maybe_finish_next_locked(dns_resolver* r) { @@ -251,6 +282,14 @@ static grpc_resolver* dns_create(grpc_resolver_args* args, .set_jitter(GRPC_DNS_RECONNECT_JITTER) .set_max_backoff(GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000); r->backoff.Init(grpc_core::BackOff(backoff_options)); + const grpc_arg* period_arg = grpc_channel_args_find( + args->args, GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS); + r->min_time_between_resolutions = + grpc_channel_arg_get_integer(period_arg, {1000, 0, INT_MAX}); + r->last_resolution_timestamp = -1; + GRPC_CLOSURE_INIT(&r->next_resolution_closure, + dns_on_next_resolution_timer_locked, r, + grpc_combiner_scheduler(r->base.combiner)); return &r->base; } diff --git a/test/core/client_channel/resolvers/BUILD b/test/core/client_channel/resolvers/BUILD index b5269c7ef0..d8b0395846 100644 --- a/test/core/client_channel/resolvers/BUILD +++ b/test/core/client_channel/resolvers/BUILD @@ -43,6 +43,18 @@ grpc_cc_test( ) grpc_cc_test( + name = "dns_resolver_cooldown_test", + srcs = ["dns_resolver_cooldown_test.cc"], + language = "C++", + deps = [ + "//:gpr", + "//:grpc", + "//test/core/util:gpr_test_util", + "//test/core/util:grpc_test_util", + ], +) + +grpc_cc_test( name = "sockaddr_resolver_test", srcs = ["sockaddr_resolver_test.cc"], language = "C++", diff --git a/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc b/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc new file mode 100644 index 0000000000..64342b48c8 --- /dev/null +++ b/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc @@ -0,0 +1,304 @@ +/* + * + * Copyright 2015 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include <cstring> + +#include <grpc/support/log.h> + +#include "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h" +#include "src/core/ext/filters/client_channel/resolver_registry.h" +#include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/iomgr/combiner.h" +#include "src/core/lib/iomgr/sockaddr_utils.h" +#include "test/core/util/test_config.h" + +static grpc_combiner* g_combiner; + +static void (*g_default_grpc_resolve_address)( + const char* name, const char* default_port, + grpc_pollset_set* interested_parties, grpc_closure* on_done, + grpc_resolved_addresses** addrs); + +grpc_ares_request* (*g_default_dns_lookup_ares)( + const char* dns_server, const char* name, const char* default_port, + grpc_pollset_set* interested_parties, grpc_closure* on_done, + grpc_lb_addresses** addrs, bool check_grpclb, char** service_config_json); + +// Counter incremented by test_resolve_address_impl indicating the number of +// times a system-level resolution has happened. +static int g_resolution_count; + +struct iomgr_args { + gpr_event ev; + gpr_atm done_atm; + gpr_mu* mu; + grpc_pollset* pollset; + grpc_pollset_set* pollset_set; +} g_iomgr_args; + +// Wrapper around g_default_grpc_resolve_address in order to count the number of +// times we incur in a system-level name resolution. +static void test_resolve_address_impl(const char* name, + const char* default_port, + grpc_pollset_set* interested_parties, + grpc_closure* on_done, + grpc_resolved_addresses** addrs) { + g_default_grpc_resolve_address(name, default_port, g_iomgr_args.pollset_set, + on_done, addrs); + ++g_resolution_count; +} + +grpc_ares_request* test_dns_lookup_ares( + const char* dns_server, const char* name, const char* default_port, + grpc_pollset_set* interested_parties, grpc_closure* on_done, + grpc_lb_addresses** addrs, bool check_grpclb, char** service_config_json) { + grpc_ares_request* result = g_default_dns_lookup_ares( + dns_server, name, default_port, g_iomgr_args.pollset_set, on_done, addrs, + check_grpclb, service_config_json); + ++g_resolution_count; + return result; +} + +static gpr_timespec test_deadline(void) { + return grpc_timeout_seconds_to_deadline(100); +} + +static void do_nothing(void* arg, grpc_error* error) {} + +void iomgr_args_init(iomgr_args* args) { + gpr_event_init(&args->ev); + args->pollset = static_cast<grpc_pollset*>(gpr_zalloc(grpc_pollset_size())); + grpc_pollset_init(args->pollset, &args->mu); + args->pollset_set = grpc_pollset_set_create(); + grpc_pollset_set_add_pollset(args->pollset_set, args->pollset); + gpr_atm_rel_store(&args->done_atm, 0); +} + +void iomgr_args_finish(iomgr_args* args) { + GPR_ASSERT(gpr_event_wait(&args->ev, test_deadline())); + grpc_pollset_set_del_pollset(args->pollset_set, args->pollset); + grpc_pollset_set_destroy(args->pollset_set); + grpc_closure do_nothing_cb; + GRPC_CLOSURE_INIT(&do_nothing_cb, do_nothing, nullptr, + grpc_schedule_on_exec_ctx); + gpr_mu_lock(args->mu); + grpc_pollset_shutdown(args->pollset, &do_nothing_cb); + gpr_mu_unlock(args->mu); + // exec_ctx needs to be flushed before calling grpc_pollset_destroy() + grpc_core::ExecCtx::Get()->Flush(); + grpc_pollset_destroy(args->pollset); + gpr_free(args->pollset); +} + +static grpc_millis n_sec_deadline(int seconds) { + return grpc_timespec_to_millis_round_up( + grpc_timeout_seconds_to_deadline(seconds)); +} + +static void poll_pollset_until_request_done(iomgr_args* args) { + grpc_core::ExecCtx exec_ctx; + grpc_millis deadline = n_sec_deadline(10); + while (true) { + bool done = gpr_atm_acq_load(&args->done_atm) != 0; + if (done) { + break; + } + grpc_millis time_left = deadline - grpc_core::ExecCtx::Get()->Now(); + gpr_log(GPR_DEBUG, "done=%d, time_left=%" PRIdPTR, done, time_left); + GPR_ASSERT(time_left >= 0); + grpc_pollset_worker* worker = nullptr; + gpr_mu_lock(args->mu); + GRPC_LOG_IF_ERROR("pollset_work", grpc_pollset_work(args->pollset, &worker, + n_sec_deadline(1))); + gpr_mu_unlock(args->mu); + grpc_core::ExecCtx::Get()->Flush(); + } + gpr_event_set(&args->ev, (void*)1); +} + +typedef struct on_resolution_cb_arg { + const char* uri_str; + grpc_resolver* resolver; + grpc_channel_args* result; + grpc_millis delay_before_second_resolution; + bool using_cares; +} on_resolution_cb_arg; + +// Counter for the number of times a resolution notification callback has been +// invoked. +static int g_on_resolution_invocations_count; + +// Set to true by the last callback in the resolution chain. +bool g_all_callbacks_invoked; + +void on_third_resolution(void* arg, grpc_error* error) { + on_resolution_cb_arg* cb_arg = static_cast<on_resolution_cb_arg*>(arg); + GPR_ASSERT(error == GRPC_ERROR_NONE); + ++g_on_resolution_invocations_count; + grpc_channel_args_destroy(cb_arg->result); + gpr_log(GPR_INFO, + "3rd: g_on_resolution_invocations_count: %d, g_resolution_count: %d", + g_on_resolution_invocations_count, g_resolution_count); + // In this case we expect to have incurred in another system-level resolution + // because on_second_resolution slept for longer than the min resolution + // period. + GPR_ASSERT(g_on_resolution_invocations_count == 3); + GPR_ASSERT(g_resolution_count == 2); + grpc_resolver_shutdown_locked(cb_arg->resolver); + GRPC_RESOLVER_UNREF(cb_arg->resolver, "on_third_resolution"); + if (cb_arg->using_cares) { + gpr_atm_rel_store(&g_iomgr_args.done_atm, 1); + gpr_mu_lock(g_iomgr_args.mu); + GRPC_LOG_IF_ERROR("pollset_kick", + grpc_pollset_kick(g_iomgr_args.pollset, nullptr)); + gpr_mu_unlock(g_iomgr_args.mu); + } + gpr_free(cb_arg); + g_all_callbacks_invoked = true; +} + +void on_second_resolution(void* arg, grpc_error* error) { + on_resolution_cb_arg* cb_arg = static_cast<on_resolution_cb_arg*>(arg); + ++g_on_resolution_invocations_count; + grpc_channel_args_destroy(cb_arg->result); + + gpr_log(GPR_INFO, + "2nd: g_on_resolution_invocations_count: %d, g_resolution_count: %d", + g_on_resolution_invocations_count, g_resolution_count); + // The resolution request for which this function is the callback happened + // before the min resolution period. Therefore, no new system-level + // resolutions happened, as indicated by g_resolution_count. + GPR_ASSERT(g_on_resolution_invocations_count == 2); + GPR_ASSERT(g_resolution_count == 1); + grpc_core::ExecCtx::Get()->TestOnlySetNow( + cb_arg->delay_before_second_resolution * 2); + grpc_resolver_next_locked( + cb_arg->resolver, &cb_arg->result, + GRPC_CLOSURE_CREATE(on_third_resolution, arg, + grpc_combiner_scheduler(g_combiner))); + grpc_resolver_channel_saw_error_locked(cb_arg->resolver); + if (cb_arg->using_cares) { + gpr_mu_lock(g_iomgr_args.mu); + GRPC_LOG_IF_ERROR("pollset_kick", + grpc_pollset_kick(g_iomgr_args.pollset, nullptr)); + gpr_mu_unlock(g_iomgr_args.mu); + } +} + +void on_first_resolution(void* arg, grpc_error* error) { + on_resolution_cb_arg* cb_arg = static_cast<on_resolution_cb_arg*>(arg); + ++g_on_resolution_invocations_count; + grpc_channel_args_destroy(cb_arg->result); + grpc_resolver_next_locked( + cb_arg->resolver, &cb_arg->result, + GRPC_CLOSURE_CREATE(on_second_resolution, arg, + grpc_combiner_scheduler(g_combiner))); + grpc_resolver_channel_saw_error_locked(cb_arg->resolver); + gpr_log(GPR_INFO, + "1st: g_on_resolution_invocations_count: %d, g_resolution_count: %d", + g_on_resolution_invocations_count, g_resolution_count); + // Theres one initial system-level resolution and one invocation of a + // notification callback (the current function). + GPR_ASSERT(g_on_resolution_invocations_count == 1); + GPR_ASSERT(g_resolution_count == 1); + if (cb_arg->using_cares) { + gpr_mu_lock(g_iomgr_args.mu); + GRPC_LOG_IF_ERROR("pollset_kick", + grpc_pollset_kick(g_iomgr_args.pollset, nullptr)); + gpr_mu_unlock(g_iomgr_args.mu); + } +} + +static void start_test_under_combiner(void* arg, grpc_error* error) { + on_resolution_cb_arg* res_cb_arg = static_cast<on_resolution_cb_arg*>(arg); + grpc_resolver* resolver; + grpc_resolver_factory* factory = grpc_resolver_factory_lookup("dns"); + grpc_uri* uri = grpc_uri_parse(res_cb_arg->uri_str, 0); + grpc_resolver_args args; + gpr_log(GPR_DEBUG, "test: '%s' should be valid for '%s'", res_cb_arg->uri_str, + factory->vtable->scheme); + GPR_ASSERT(uri); + memset(&args, 0, sizeof(args)); + args.uri = uri; + args.combiner = g_combiner; + g_on_resolution_invocations_count = 0; + g_resolution_count = 0; + constexpr int kMinResolutionPeriodMs = 1000; + + grpc_arg cooldown_arg; + cooldown_arg.key = + const_cast<char*>(GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS); + cooldown_arg.type = GRPC_ARG_INTEGER; + cooldown_arg.value.integer = kMinResolutionPeriodMs; + auto* cooldown_channel_args = + grpc_channel_args_copy_and_add(nullptr, &cooldown_arg, 1); + args.args = cooldown_channel_args; + resolver = grpc_resolver_factory_create_resolver(factory, &args); + grpc_channel_args_destroy(cooldown_channel_args); + GPR_ASSERT(resolver != nullptr); + res_cb_arg->resolver = resolver; + res_cb_arg->delay_before_second_resolution = kMinResolutionPeriodMs; + // First resolution, would incur in system-level resolution. + grpc_resolver_next_locked( + resolver, &res_cb_arg->result, + GRPC_CLOSURE_CREATE(on_first_resolution, res_cb_arg, + grpc_combiner_scheduler(g_combiner))); + grpc_uri_destroy(uri); + grpc_resolver_factory_unref(factory); +} + +static void test_cooldown(bool using_cares) { + grpc_core::ExecCtx exec_ctx; + if (using_cares) iomgr_args_init(&g_iomgr_args); + on_resolution_cb_arg* res_cb_arg = + static_cast<on_resolution_cb_arg*>(gpr_zalloc(sizeof(*res_cb_arg))); + res_cb_arg->uri_str = "dns:127.0.0.1"; + res_cb_arg->using_cares = using_cares; + + GRPC_CLOSURE_SCHED(GRPC_CLOSURE_CREATE(start_test_under_combiner, res_cb_arg, + grpc_combiner_scheduler(g_combiner)), + GRPC_ERROR_NONE); + if (using_cares) { + grpc_core::ExecCtx::Get()->Flush(); + poll_pollset_until_request_done(&g_iomgr_args); + iomgr_args_finish(&g_iomgr_args); + } +} + +int main(int argc, char** argv) { + grpc_test_init(argc, argv); + grpc_init(); + + g_combiner = grpc_combiner_create(); + + const bool using_cares = (grpc_resolve_address == grpc_resolve_address_ares); + g_default_grpc_resolve_address = grpc_resolve_address; + g_default_dns_lookup_ares = grpc_dns_lookup_ares; + grpc_dns_lookup_ares = test_dns_lookup_ares; + grpc_resolve_address = test_resolve_address_impl; + + test_cooldown(using_cares); + + { + grpc_core::ExecCtx exec_ctx; + GRPC_COMBINER_UNREF(g_combiner, "test"); + } + grpc_shutdown(); + GPR_ASSERT(g_all_callbacks_invoked); + return 0; +} diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 40142c02fd..06992cbda8 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -359,6 +359,23 @@ "headers": [], "is_filegroup": false, "language": "c", + "name": "dns_resolver_cooldown_test", + "src": [ + "test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc" + ], + "third_party": false, + "type": "target" + }, + { + "deps": [ + "gpr", + "gpr_test_util", + "grpc", + "grpc_test_util" + ], + "headers": [], + "is_filegroup": false, + "language": "c", "name": "dns_resolver_test", "src": [ "test/core/client_channel/resolvers/dns_resolver_test.cc" diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index cbb2428680..7c7de0a94b 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -454,6 +454,30 @@ "flaky": false, "gtest": false, "language": "c", + "name": "dns_resolver_cooldown_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": true + }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": false, + "language": "c", "name": "dns_resolver_test", "platforms": [ "linux", |