Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #19489 from apolcyn/fix_ares_throttle

Browse Source 
Fix DNS resolver cooldown
pull/19503/head
apolcyn 5 years ago committed by GitHub
parent 3a33fccb3a 0d2c622f9e
commit 58ed35407a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 208 additions and 33 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 47
      CMakeLists.txt
  2. 58
      Makefile
  3. 15
      build.yaml
  4. 3
      src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc
  5. 3
      src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc
  6. 19
      test/core/client_channel/resolvers/BUILD
  7. 46
      test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc
  8. 18
      tools/run_tests/generated/sources_and_headers.json
  9. 32
      tools/run_tests/generated/tests.json

47
CMakeLists.txt
Unescape View File

@ -275,7 +275,8 @@ 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_cooldown_using_ares_resolver_test)
add_dependencies(buildtests_c dns_resolver_cooldown_using_native_resolver_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)
@ -6877,12 +6878,12 @@ target_link_libraries(dns_resolver_connectivity_test
endif (gRPC_BUILD_TESTS)
if (gRPC_BUILD_TESTS)
add_executable(dns_resolver_cooldown_test
add_executable(dns_resolver_cooldown_using_ares_resolver_test
test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc
)
target_include_directories(dns_resolver_cooldown_test
target_include_directories(dns_resolver_cooldown_using_ares_resolver_test
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include
PRIVATE ${_gRPC_SSL_INCLUDE_DIR}
@ -6895,7 +6896,7 @@ target_include_directories(dns_resolver_cooldown_test
PRIVATE ${_gRPC_NANOPB_INCLUDE_DIR}
)
target_link_libraries(dns_resolver_cooldown_test
target_link_libraries(dns_resolver_cooldown_using_ares_resolver_test
${_gRPC_ALLTARGETS_LIBRARIES}
grpc_test_util
grpc
@ -6904,8 +6905,42 @@ target_link_libraries(dns_resolver_cooldown_test
# avoid dependency on libstdc++
if (_gRPC_CORE_NOSTDCXX_FLAGS)
set_target_properties(dns_resolver_cooldown_test PROPERTIES LINKER_LANGUAGE C)
target_compile_options(dns_resolver_cooldown_test PRIVATE $<$<COMPILE_LANGUAGE:CXX>:${_gRPC_CORE_NOSTDCXX_FLAGS}>)
set_target_properties(dns_resolver_cooldown_using_ares_resolver_test PROPERTIES LINKER_LANGUAGE C)
target_compile_options(dns_resolver_cooldown_using_ares_resolver_test PRIVATE $<$<COMPILE_LANGUAGE:CXX>:${_gRPC_CORE_NOSTDCXX_FLAGS}>)
endif()
endif (gRPC_BUILD_TESTS)
if (gRPC_BUILD_TESTS)
add_executable(dns_resolver_cooldown_using_native_resolver_test
test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc
)
target_include_directories(dns_resolver_cooldown_using_native_resolver_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}
PRIVATE ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
PRIVATE ${_gRPC_NANOPB_INCLUDE_DIR}
)
target_link_libraries(dns_resolver_cooldown_using_native_resolver_test
${_gRPC_ALLTARGETS_LIBRARIES}
grpc_test_util
grpc
gpr
)
# avoid dependency on libstdc++
if (_gRPC_CORE_NOSTDCXX_FLAGS)
set_target_properties(dns_resolver_cooldown_using_native_resolver_test PROPERTIES LINKER_LANGUAGE C)
target_compile_options(dns_resolver_cooldown_using_native_resolver_test PRIVATE $<$<COMPILE_LANGUAGE:CXX>:${_gRPC_CORE_NOSTDCXX_FLAGS}>)
endif()
endif (gRPC_BUILD_TESTS)

58
Makefile
Unescape View File

@ -1015,7 +1015,8 @@ 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_cooldown_using_ares_resolver_test: $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_ares_resolver_test
dns_resolver_cooldown_using_native_resolver_test: $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_native_resolver_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
@ -1446,7 +1447,8 @@ 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_cooldown_using_ares_resolver_test \
$(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_native_resolver_test \
$(BINDIR)/$(CONFIG)/dns_resolver_test \
$(BINDIR)/$(CONFIG)/dualstack_socket_test \
$(BINDIR)/$(CONFIG)/endpoint_pair_test \
@ -1983,8 +1985,10 @@ 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_cooldown_using_ares_resolver_test"
$(Q) $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_ares_resolver_test || ( echo test dns_resolver_cooldown_using_ares_resolver_test failed ; exit 1 )
$(E) "[RUN] Testing dns_resolver_cooldown_using_native_resolver_test"
$(Q) $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_native_resolver_test || ( echo test dns_resolver_cooldown_using_native_resolver_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"
@ -9641,34 +9645,66 @@ endif
endif
DNS_RESOLVER_COOLDOWN_TEST_SRC = \
DNS_RESOLVER_COOLDOWN_USING_ARES_RESOLVER_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))))
DNS_RESOLVER_COOLDOWN_USING_ARES_RESOLVER_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(DNS_RESOLVER_COOLDOWN_USING_ARES_RESOLVER_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
$(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_ares_resolver_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.a
$(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_ares_resolver_test: $(DNS_RESOLVER_COOLDOWN_USING_ARES_RESOLVER_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.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.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_test
$(Q) $(LD) $(LDFLAGS) $(DNS_RESOLVER_COOLDOWN_USING_ARES_RESOLVER_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_ares_resolver_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.a
deps_dns_resolver_cooldown_test: $(DNS_RESOLVER_COOLDOWN_TEST_OBJS:.o=.dep)
deps_dns_resolver_cooldown_using_ares_resolver_test: $(DNS_RESOLVER_COOLDOWN_USING_ARES_RESOLVER_TEST_OBJS:.o=.dep)
ifneq ($(NO_SECURE),true)
ifneq ($(NO_DEPS),true)
-include $(DNS_RESOLVER_COOLDOWN_TEST_OBJS:.o=.dep)
-include $(DNS_RESOLVER_COOLDOWN_USING_ARES_RESOLVER_TEST_OBJS:.o=.dep)
endif
endif
DNS_RESOLVER_COOLDOWN_USING_NATIVE_RESOLVER_TEST_SRC = \
test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc \
DNS_RESOLVER_COOLDOWN_USING_NATIVE_RESOLVER_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(DNS_RESOLVER_COOLDOWN_USING_NATIVE_RESOLVER_TEST_SRC))))
ifeq ($(NO_SECURE),true)
# You can't build secure targets if you don't have OpenSSL.
$(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_native_resolver_test: openssl_dep_error
else
$(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_native_resolver_test: $(DNS_RESOLVER_COOLDOWN_USING_NATIVE_RESOLVER_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a
$(E) "[LD] Linking $@"
$(Q) mkdir -p `dirname $@`
$(Q) $(LD) $(LDFLAGS) $(DNS_RESOLVER_COOLDOWN_USING_NATIVE_RESOLVER_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/dns_resolver_cooldown_using_native_resolver_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.a
deps_dns_resolver_cooldown_using_native_resolver_test: $(DNS_RESOLVER_COOLDOWN_USING_NATIVE_RESOLVER_TEST_OBJS:.o=.dep)
ifneq ($(NO_SECURE),true)
ifneq ($(NO_DEPS),true)
-include $(DNS_RESOLVER_COOLDOWN_USING_NATIVE_RESOLVER_TEST_OBJS:.o=.dep)
endif
endif

15
build.yaml
Unescape View File

@ -2400,7 +2400,7 @@ targets:
- gpr
exclude_iomgrs:
- uv
- name: dns_resolver_cooldown_test
- name: dns_resolver_cooldown_using_ares_resolver_test
build: test
language: c
src:
@ -2409,6 +2409,19 @@ targets:
- grpc_test_util
- grpc
- gpr
args:
- --resolver=ares
- name: dns_resolver_cooldown_using_native_resolver_test
build: test
language: c
src:
- test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc
deps:
- grpc_test_util
- grpc
- gpr
args:
- --resolver=native
- name: dns_resolver_test
build: test
language: c

3
src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc
Unescape View File

@ -401,7 +401,8 @@ void AresDnsResolver::MaybeStartResolvingLocked() {
// new closure API is done, find a way to track this ref with the timer
// callback as part of the type system.
Ref(DEBUG_LOCATION, "next_resolution_timer_cooldown").release();
grpc_timer_init(&next_resolution_timer_, ms_until_next_resolution,
grpc_timer_init(&next_resolution_timer_,
ExecCtx::Get()->Now() + ms_until_next_resolution,
&on_next_resolution_);
return;
}

3
src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc
Unescape View File

@ -230,7 +230,8 @@ void NativeDnsResolver::MaybeStartResolvingLocked() {
// new closure API is done, find a way to track this ref with the timer
// callback as part of the type system.
Ref(DEBUG_LOCATION, "next_resolution_timer_cooldown").release();
grpc_timer_init(&next_resolution_timer_, ms_until_next_resolution,
grpc_timer_init(&next_resolution_timer_,
ExecCtx::Get()->Now() + ms_until_next_resolution,
&on_next_resolution_);
return;
}

19
test/core/client_channel/resolvers/BUILD
Unescape View File

@ -19,9 +19,26 @@ grpc_package(name = "test/core/client_channel_resolvers")
licenses(["notice"]) # Apache v2
grpc_cc_test(
name = "dns_resolver_connectivity_test",
name = "dns_resolver_connectivity_using_ares_resolver_test",
srcs = ["dns_resolver_connectivity_test.cc"],
language = "C++",
args = [
"--resolver=ares",
],
deps = [
"//:gpr",
"//:grpc",
"//test/core/util:grpc_test_util",
],
)
grpc_cc_test(
name = "dns_resolver_connectivity_using_native_resolver_test",
srcs = ["dns_resolver_connectivity_test.cc"],
language = "C++",
args = [
"--resolver=native",
],
deps = [
"//:gpr",
"//:grpc",

46
test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc
Unescape View File

@ -101,12 +101,16 @@ static grpc_ares_request* test_dns_lookup_ares_locked(
addresses, check_grpclb, service_config_json, query_timeout_ms, combiner);
++g_resolution_count;
static grpc_millis last_resolution_time = 0;
grpc_millis now =
grpc_timespec_to_millis_round_up(gpr_now(GPR_CLOCK_MONOTONIC));
gpr_log(GPR_DEBUG,
"last_resolution_time:%" PRId64 " now:%" PRId64
" min_time_between:%d",
last_resolution_time, now, kMinResolutionPeriodForCheckMs);
if (last_resolution_time == 0) {
last_resolution_time =
grpc_timespec_to_millis_round_up(gpr_now(GPR_CLOCK_MONOTONIC));
} else {
grpc_millis now =
grpc_timespec_to_millis_round_up(gpr_now(GPR_CLOCK_MONOTONIC));
GPR_ASSERT(now - last_resolution_time >= kMinResolutionPeriodForCheckMs);
last_resolution_time = now;
}
@ -212,19 +216,46 @@ struct OnResolutionCallbackArg {
// Set to true by the last callback in the resolution chain.
static bool g_all_callbacks_invoked;
// It's interesting to run a few rounds of this test because as
// we run more rounds, the base starting time
// (i.e. ExecCtx g_start_time) gets further and further away
// from "Now()". Thus the more rounds ran, the more highlighted the
// difference is between absolute and relative times values.
static void on_fourth_resolution(OnResolutionCallbackArg* cb_arg) {
gpr_log(GPR_INFO, "4th: g_resolution_count: %d", g_resolution_count);
GPR_ASSERT(g_resolution_count == 4);
cb_arg->resolver.reset();
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);
grpc_core::Delete(cb_arg);
g_all_callbacks_invoked = true;
}
static void on_third_resolution(OnResolutionCallbackArg* cb_arg) {
gpr_log(GPR_INFO, "3rd: g_resolution_count: %d", g_resolution_count);
GPR_ASSERT(g_resolution_count == 3);
cb_arg->result_handler->SetCallback(on_fourth_resolution, cb_arg);
cb_arg->resolver->RequestReresolutionLocked();
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 on_second_resolution(OnResolutionCallbackArg* cb_arg) {
gpr_log(GPR_INFO, "2nd: g_resolution_count: %d", g_resolution_count);
// The resolution callback was not invoked until new data was
// available, which was delayed until after the cooldown period.
GPR_ASSERT(g_resolution_count == 2);
cb_arg->resolver.reset();
gpr_atm_rel_store(&g_iomgr_args.done_atm, 1);
cb_arg->result_handler->SetCallback(on_third_resolution, cb_arg);
cb_arg->resolver->RequestReresolutionLocked();
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);
grpc_core::Delete(cb_arg);
g_all_callbacks_invoked = true;
}
static void on_first_resolution(OnResolutionCallbackArg* cb_arg) {
@ -243,9 +274,7 @@ static void on_first_resolution(OnResolutionCallbackArg* cb_arg) {
static void start_test_under_combiner(void* arg, grpc_error* error) {
OnResolutionCallbackArg* res_cb_arg =
static_cast<OnResolutionCallbackArg*>(arg);
res_cb_arg->result_handler = grpc_core::New<ResultHandler>();
grpc_core::ResolverFactory* factory =
grpc_core::ResolverRegistry::LookupResolverFactory("dns");
grpc_uri* uri = grpc_uri_parse(res_cb_arg->uri_str, 0);
@ -300,7 +329,6 @@ int main(int argc, char** argv) {
grpc_set_resolver_impl(&test_resolver);
test_cooldown();
{
grpc_core::ExecCtx exec_ctx;
GRPC_COMBINER_UNREF(g_combiner, "test");

18
tools/run_tests/generated/sources_and_headers.json
Unescape View File

@ -392,7 +392,23 @@
"headers": [],
"is_filegroup": false,
"language": "c",
"name": "dns_resolver_cooldown_test",
"name": "dns_resolver_cooldown_using_ares_resolver_test",
"src": [
"test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc"
],
"third_party": false,
"type": "target"
},
{
"deps": [
"gpr",
"grpc",
"grpc_test_util"
],
"headers": [],
"is_filegroup": false,
"language": "c",
"name": "dns_resolver_cooldown_using_native_resolver_test",
"src": [
"test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc"
],

32
tools/run_tests/generated/tests.json
Unescape View File

@ -484,7 +484,35 @@
"uses_polling": true
},
{
"args": [],
"args": [
"--resolver=ares"
],
"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_cooldown_using_ares_resolver_test",
"platforms": [
"linux",
"mac",
"posix",
"windows"
],
"uses_polling": true
},
{
"args": [
"--resolver=native"
],
"benchmark": false,
"ci_platforms": [
"linux",
@ -498,7 +526,7 @@
"flaky": false,
"gtest": false,
"language": "c",
"name": "dns_resolver_cooldown_test",
"name": "dns_resolver_cooldown_using_native_resolver_test",
"platforms": [
"linux",
"mac",

Write Preview
Loading…
Cancel
Save