From ba878c804d370d1a28d668fe62a02fca62093d65 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 21 Jun 2023 15:49:35 -0700 Subject: [PATCH] Revert "[HTTP2] Fix inconsistencies in keepalive configuration (#33428)" (#33512) This reverts commit e107ff5e99fe1b34cbc80b4bb595c9a64130cfd5. --- CMakeLists.txt | 50 ----- build_autogenerated.yaml | 35 ---- .../chttp2/transport/chttp2_transport.cc | 187 +++++++++++------- .../ext/transport/chttp2/transport/internal.h | 2 - test/core/transport/chttp2/BUILD | 14 -- .../chttp2/ping_configuration_test.cc | 174 ---------------- tools/run_tests/generated/tests.json | 24 --- 7 files changed, 115 insertions(+), 371 deletions(-) delete mode 100644 test/core/transport/chttp2/ping_configuration_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 437a132db70..d526b190dde 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1137,7 +1137,6 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx periodic_update_test) add_dependencies(buildtests_cxx pick_first_test) add_dependencies(buildtests_cxx pid_controller_test) - add_dependencies(buildtests_cxx ping_configuration_test) add_dependencies(buildtests_cxx ping_pong_streaming_test) add_dependencies(buildtests_cxx ping_test) add_dependencies(buildtests_cxx pipe_test) @@ -18237,55 +18236,6 @@ target_link_libraries(pid_controller_test ) -endif() -if(gRPC_BUILD_TESTS) - -add_executable(ping_configuration_test - test/core/transport/chttp2/ping_configuration_test.cc - test/core/util/cmdline.cc - test/core/util/fuzzer_util.cc - test/core/util/grpc_profiler.cc - test/core/util/histogram.cc - test/core/util/mock_endpoint.cc - test/core/util/parse_hexstring.cc - test/core/util/passthru_endpoint.cc - test/core/util/resolve_localhost_ip46.cc - test/core/util/slice_splitter.cc - test/core/util/subprocess_posix.cc - test/core/util/subprocess_windows.cc - test/core/util/tracer_util.cc - third_party/googletest/googletest/src/gtest-all.cc - third_party/googletest/googlemock/src/gmock-all.cc -) -target_compile_features(ping_configuration_test PUBLIC cxx_std_14) -target_include_directories(ping_configuration_test - PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR} - ${CMAKE_CURRENT_SOURCE_DIR}/include - ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} - ${_gRPC_RE2_INCLUDE_DIR} - ${_gRPC_SSL_INCLUDE_DIR} - ${_gRPC_UPB_GENERATED_DIR} - ${_gRPC_UPB_GRPC_GENERATED_DIR} - ${_gRPC_UPB_INCLUDE_DIR} - ${_gRPC_XXHASH_INCLUDE_DIR} - ${_gRPC_ZLIB_INCLUDE_DIR} - third_party/googletest/googletest/include - third_party/googletest/googletest - third_party/googletest/googlemock/include - third_party/googletest/googlemock - ${_gRPC_PROTO_GENS_DIR} -) - -target_link_libraries(ping_configuration_test - ${_gRPC_BASELIB_LIBRARIES} - ${_gRPC_PROTOBUF_LIBRARIES} - ${_gRPC_ZLIB_LIBRARIES} - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util -) - - endif() if(gRPC_BUILD_TESTS) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 469e807e374..4a26347c4b9 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -11259,41 +11259,6 @@ targets: - test/core/util/tracer_util.cc deps: - grpc_test_util -- name: ping_configuration_test - gtest: true - build: test - language: c++ - headers: - - test/core/util/cmdline.h - - test/core/util/evaluate_args_test_util.h - - test/core/util/fuzzer_util.h - - test/core/util/grpc_profiler.h - - test/core/util/histogram.h - - test/core/util/mock_authorization_endpoint.h - - test/core/util/mock_endpoint.h - - test/core/util/parse_hexstring.h - - test/core/util/passthru_endpoint.h - - test/core/util/resolve_localhost_ip46.h - - test/core/util/slice_splitter.h - - test/core/util/subprocess.h - - test/core/util/tracer_util.h - src: - - test/core/transport/chttp2/ping_configuration_test.cc - - test/core/util/cmdline.cc - - test/core/util/fuzzer_util.cc - - test/core/util/grpc_profiler.cc - - test/core/util/histogram.cc - - test/core/util/mock_endpoint.cc - - test/core/util/parse_hexstring.cc - - test/core/util/passthru_endpoint.cc - - test/core/util/resolve_localhost_ip46.cc - - test/core/util/slice_splitter.cc - - test/core/util/subprocess_posix.cc - - test/core/util/subprocess_windows.cc - - test/core/util/tracer_util.cc - deps: - - grpc_test_util - uses_polling: false - name: ping_pong_streaming_test gtest: true build: test diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 8bb037f2a98..29183815485 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -106,29 +106,34 @@ #define DEFAULT_MAX_HEADER_LIST_SIZE (16 * 1024) #define DEFAULT_MAX_HEADER_LIST_SIZE_SOFT_LIMIT (8 * 1024) +#define DEFAULT_CLIENT_KEEPALIVE_TIME_MS INT_MAX +#define DEFAULT_CLIENT_KEEPALIVE_TIMEOUT_MS 20000 // 20 seconds +#define DEFAULT_SERVER_KEEPALIVE_TIME_MS 7200000 // 2 hours +#define DEFAULT_SERVER_KEEPALIVE_TIMEOUT_MS 20000 // 20 seconds #define DEFAULT_KEEPALIVE_PERMIT_WITHOUT_CALLS false #define KEEPALIVE_TIME_BACKOFF_MULTIPLIER 2 +#define DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS 300000 // 5 minutes #define DEFAULT_MAX_PINGS_BETWEEN_DATA 2 #define DEFAULT_MAX_PING_STRIKES 2 #define DEFAULT_MAX_PENDING_INDUCED_FRAMES 10000 -static grpc_core::Duration g_default_client_keepalive_time = - grpc_core::Duration::Infinity(); -static grpc_core::Duration g_default_client_keepalive_timeout = - grpc_core::Duration::Seconds(20); -static grpc_core::Duration g_default_server_keepalive_time = - grpc_core::Duration::Hours(2); -static grpc_core::Duration g_default_server_keepalive_timeout = - grpc_core::Duration::Seconds(20); +static int g_default_client_keepalive_time_ms = + DEFAULT_CLIENT_KEEPALIVE_TIME_MS; +static int g_default_client_keepalive_timeout_ms = + DEFAULT_CLIENT_KEEPALIVE_TIMEOUT_MS; +static int g_default_server_keepalive_time_ms = + DEFAULT_SERVER_KEEPALIVE_TIME_MS; +static int g_default_server_keepalive_timeout_ms = + DEFAULT_SERVER_KEEPALIVE_TIMEOUT_MS; static bool g_default_client_keepalive_permit_without_calls = DEFAULT_KEEPALIVE_PERMIT_WITHOUT_CALLS; static bool g_default_server_keepalive_permit_without_calls = DEFAULT_KEEPALIVE_PERMIT_WITHOUT_CALLS; -static grpc_core::Duration g_default_min_recv_ping_interval_without_data = - grpc_core::Duration::Minutes(5); +static int g_default_min_recv_ping_interval_without_data_ms = + DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS; static int g_default_max_pings_without_data = DEFAULT_MAX_PINGS_BETWEEN_DATA; static int g_default_max_ping_strikes = DEFAULT_MAX_PING_STRIKES; @@ -364,25 +369,26 @@ static void read_channel_args(grpc_chttp2_transport* t, channel_args .GetDurationFromIntMillis( GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS) - .value_or(g_default_min_recv_ping_interval_without_data)); + .value_or(grpc_core::Duration::Milliseconds( + g_default_min_recv_ping_interval_without_data_ms))); t->write_buffer_size = std::max(0, channel_args.GetInt(GRPC_ARG_HTTP2_WRITE_BUFFER_SIZE) .value_or(grpc_core::chttp2::kDefaultWindow)); t->keepalive_time = std::max(grpc_core::Duration::Milliseconds(1), channel_args.GetDurationFromIntMillis(GRPC_ARG_KEEPALIVE_TIME_MS) - .value_or(t->is_client ? g_default_client_keepalive_time - : g_default_server_keepalive_time)); + .value_or(grpc_core::Duration::Milliseconds( + t->is_client ? g_default_client_keepalive_time_ms + : g_default_server_keepalive_time_ms))); t->keepalive_timeout = std::max( grpc_core::Duration::Zero(), channel_args.GetDurationFromIntMillis(GRPC_ARG_KEEPALIVE_TIMEOUT_MS) - .value_or(t->is_client ? g_default_client_keepalive_timeout - : g_default_server_keepalive_timeout)); + .value_or(grpc_core::Duration::Milliseconds( + t->is_client ? g_default_client_keepalive_timeout_ms + : g_default_server_keepalive_timeout_ms))); t->keepalive_permit_without_calls = channel_args.GetBool(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS) - .value_or(t->is_client - ? g_default_client_keepalive_permit_without_calls - : g_default_server_keepalive_permit_without_calls); + .value_or(false); // Only send the prefered rx frame size http2 setting if we are instructed // to auto size the buffers allocated at tcp level and we also can adjust // sending frame size. @@ -506,6 +512,40 @@ static void read_channel_args(grpc_chttp2_transport* t, } } +static void init_transport_keepalive_settings(grpc_chttp2_transport* t) { + if (t->is_client) { + t->keepalive_time = g_default_client_keepalive_time_ms == INT_MAX + ? grpc_core::Duration::Infinity() + : grpc_core::Duration::Milliseconds( + g_default_client_keepalive_time_ms); + t->keepalive_timeout = g_default_client_keepalive_timeout_ms == INT_MAX + ? grpc_core::Duration::Infinity() + : grpc_core::Duration::Milliseconds( + g_default_client_keepalive_timeout_ms); + t->keepalive_permit_without_calls = + g_default_client_keepalive_permit_without_calls; + } else { + t->keepalive_time = g_default_server_keepalive_time_ms == INT_MAX + ? grpc_core::Duration::Infinity() + : grpc_core::Duration::Milliseconds( + g_default_server_keepalive_time_ms); + t->keepalive_timeout = g_default_server_keepalive_timeout_ms == INT_MAX + ? grpc_core::Duration::Infinity() + : grpc_core::Duration::Milliseconds( + g_default_server_keepalive_timeout_ms); + t->keepalive_permit_without_calls = + g_default_server_keepalive_permit_without_calls; + } +} + +static void configure_transport_ping_policy(grpc_chttp2_transport* t) { + t->ping_policy.max_pings_without_data = g_default_max_pings_without_data; + t->ping_policy.max_ping_strikes = g_default_max_ping_strikes; + t->ping_policy.min_recv_ping_interval_without_data = + grpc_core::Duration::Milliseconds( + g_default_min_recv_ping_interval_without_data_ms); +} + static void init_keepalive_pings_if_enabled_locked( void* arg, GRPC_UNUSED grpc_error_handle error) { GPR_DEBUG_ASSERT(error.ok()); @@ -592,6 +632,9 @@ grpc_chttp2_transport::grpc_chttp2_transport( queue_setting_update(this, GRPC_CHTTP2_SETTINGS_GRPC_ALLOW_TRUE_BINARY_METADATA, 1); + configure_transport_ping_policy(this); + init_transport_keepalive_settings(this); + read_channel_args(this, channel_args, is_client); // No pings allowed before receiving a header or data frame. @@ -2709,61 +2752,61 @@ static void next_bdp_ping_timer_expired_locked( void grpc_chttp2_config_default_keepalive_args(grpc_channel_args* args, bool is_client) { - grpc_chttp2_config_default_keepalive_args(grpc_core::ChannelArgs::FromC(args), - is_client); -} - -void grpc_chttp2_config_default_keepalive_args( - const grpc_core::ChannelArgs& channel_args, bool is_client) { - const auto keepalive_time = - std::max(grpc_core::Duration::Milliseconds(1), - channel_args.GetDurationFromIntMillis(GRPC_ARG_KEEPALIVE_TIME_MS) - .value_or(is_client ? g_default_client_keepalive_time - : g_default_server_keepalive_time)); - if (is_client) { - g_default_client_keepalive_time = keepalive_time; - } else { - g_default_server_keepalive_time = keepalive_time; - } - - const auto keepalive_timeout = std::max( - grpc_core::Duration::Zero(), - channel_args.GetDurationFromIntMillis(GRPC_ARG_KEEPALIVE_TIMEOUT_MS) - .value_or(is_client ? g_default_client_keepalive_timeout - : g_default_server_keepalive_timeout)); - if (is_client) { - g_default_client_keepalive_timeout = keepalive_timeout; - } else { - g_default_server_keepalive_timeout = keepalive_timeout; - } - - const bool keepalive_permit_without_calls = - channel_args.GetBool(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS) - .value_or(is_client - ? g_default_client_keepalive_permit_without_calls - : g_default_server_keepalive_permit_without_calls); - if (is_client) { - g_default_client_keepalive_permit_without_calls = - keepalive_permit_without_calls; - } else { - g_default_server_keepalive_permit_without_calls = - keepalive_permit_without_calls; + size_t i; + if (args) { + for (i = 0; i < args->num_args; i++) { + if (0 == strcmp(args->args[i].key, GRPC_ARG_KEEPALIVE_TIME_MS)) { + const int value = grpc_channel_arg_get_integer( + &args->args[i], {is_client ? g_default_client_keepalive_time_ms + : g_default_server_keepalive_time_ms, + 1, INT_MAX}); + if (is_client) { + g_default_client_keepalive_time_ms = value; + } else { + g_default_server_keepalive_time_ms = value; + } + } else if (0 == + strcmp(args->args[i].key, GRPC_ARG_KEEPALIVE_TIMEOUT_MS)) { + const int value = grpc_channel_arg_get_integer( + &args->args[i], {is_client ? g_default_client_keepalive_timeout_ms + : g_default_server_keepalive_timeout_ms, + 0, INT_MAX}); + if (is_client) { + g_default_client_keepalive_timeout_ms = value; + } else { + g_default_server_keepalive_timeout_ms = value; + } + } else if (0 == strcmp(args->args[i].key, + GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS)) { + const bool value = static_cast(grpc_channel_arg_get_integer( + &args->args[i], + {is_client ? g_default_client_keepalive_permit_without_calls + : g_default_server_keepalive_timeout_ms, + 0, 1})); + if (is_client) { + g_default_client_keepalive_permit_without_calls = value; + } else { + g_default_server_keepalive_permit_without_calls = value; + } + } else if (0 == + strcmp(args->args[i].key, GRPC_ARG_HTTP2_MAX_PING_STRIKES)) { + g_default_max_ping_strikes = grpc_channel_arg_get_integer( + &args->args[i], {g_default_max_ping_strikes, 0, INT_MAX}); + } else if (0 == strcmp(args->args[i].key, + GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA)) { + g_default_max_pings_without_data = grpc_channel_arg_get_integer( + &args->args[i], {g_default_max_pings_without_data, 0, INT_MAX}); + } else if (0 == + strcmp( + args->args[i].key, + GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS)) { + g_default_min_recv_ping_interval_without_data_ms = + grpc_channel_arg_get_integer( + &args->args[i], + {g_default_min_recv_ping_interval_without_data_ms, 0, INT_MAX}); + } + } } - - g_default_max_ping_strikes = - std::max(0, channel_args.GetInt(GRPC_ARG_HTTP2_MAX_PING_STRIKES) - .value_or(g_default_max_ping_strikes)); - - g_default_max_pings_without_data = - std::max(0, channel_args.GetInt(GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA) - .value_or(g_default_max_pings_without_data)); - - g_default_min_recv_ping_interval_without_data = - std::max(grpc_core::Duration::Zero(), - channel_args - .GetDurationFromIntMillis( - GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS) - .value_or(g_default_min_recv_ping_interval_without_data)); } static void init_keepalive_ping(grpc_chttp2_transport* t) { diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index 4f1ce78d8fb..f7445d65e09 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -805,8 +805,6 @@ void grpc_chttp2_fail_pending_writes(grpc_chttp2_transport* t, /// initialization void grpc_chttp2_config_default_keepalive_args(grpc_channel_args* args, bool is_client); -void grpc_chttp2_config_default_keepalive_args( - const grpc_core::ChannelArgs& channel_args, bool is_client); void grpc_chttp2_retry_initiate_ping(grpc_chttp2_transport* t); diff --git a/test/core/transport/chttp2/BUILD b/test/core/transport/chttp2/BUILD index 66fbbf23a79..e32a07da966 100644 --- a/test/core/transport/chttp2/BUILD +++ b/test/core/transport/chttp2/BUILD @@ -153,20 +153,6 @@ grpc_cc_test( ], ) -grpc_cc_test( - name = "ping_configuration_test", - srcs = ["ping_configuration_test.cc"], - external_deps = ["gtest"], - language = "C++", - uses_polling = False, - deps = [ - "//:gpr", - "//:grpc", - "//test/core/util:grpc_test_util", - "//test/core/util:grpc_test_util_base", - ], -) - grpc_cc_test( name = "flow_control_test", srcs = ["flow_control_test.cc"], diff --git a/test/core/transport/chttp2/ping_configuration_test.cc b/test/core/transport/chttp2/ping_configuration_test.cc deleted file mode 100644 index cac4449ed3d..00000000000 --- a/test/core/transport/chttp2/ping_configuration_test.cc +++ /dev/null @@ -1,174 +0,0 @@ -// Copyright 2023 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 "gtest/gtest.h" - -#include -#include - -#include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" -#include "src/core/ext/transport/chttp2/transport/frame.h" -#include "src/core/ext/transport/chttp2/transport/internal.h" -#include "src/core/lib/channel/channel_args.h" -#include "src/core/lib/event_engine/default_event_engine.h" -#include "src/core/lib/gprpp/time.h" -#include "src/core/lib/iomgr/endpoint.h" -#include "src/core/lib/iomgr/exec_ctx.h" -#include "src/core/lib/resource_quota/resource_quota.h" -#include "src/core/lib/transport/transport.h" -#include "test/core/util/mock_endpoint.h" -#include "test/core/util/test_config.h" - -namespace grpc_core { -namespace { - -class ConfigurationTest : public ::testing::Test { - protected: - ConfigurationTest() { - mock_endpoint_ = grpc_mock_endpoint_create(DiscardWrite); - args_ = args_.SetObject(ResourceQuota::Default()); - args_ = args_.SetObject( - grpc_event_engine::experimental::GetDefaultEventEngine()); - } - - grpc_endpoint* mock_endpoint_ = nullptr; - ChannelArgs args_; - - private: - static void DiscardWrite(grpc_slice /*slice*/) {} -}; - -TEST_F(ConfigurationTest, ClientKeepaliveDefaults) { - ExecCtx exec_ctx; - grpc_chttp2_transport* t = reinterpret_cast( - grpc_create_chttp2_transport(args_, mock_endpoint_, /*is_client=*/true)); - EXPECT_EQ(t->keepalive_time, Duration::Infinity()); - EXPECT_EQ(t->keepalive_timeout, Duration::Seconds(20)); - EXPECT_EQ(t->keepalive_permit_without_calls, false); - EXPECT_EQ(t->ping_policy.max_pings_without_data, 2); - grpc_transport_destroy(&t->base); -} - -TEST_F(ConfigurationTest, ClientKeepaliveExplicitArgs) { - ExecCtx exec_ctx; - args_ = args_.Set(GRPC_ARG_KEEPALIVE_TIME_MS, 20000); - args_ = args_.Set(GRPC_ARG_KEEPALIVE_TIMEOUT_MS, 10000); - args_ = args_.Set(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS, true); - args_ = args_.Set(GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, 3); - grpc_chttp2_transport* t = reinterpret_cast( - grpc_create_chttp2_transport(args_, mock_endpoint_, /*is_client=*/true)); - EXPECT_EQ(t->keepalive_time, Duration::Seconds(20)); - EXPECT_EQ(t->keepalive_timeout, Duration::Seconds(10)); - EXPECT_EQ(t->keepalive_permit_without_calls, true); - EXPECT_EQ(t->ping_policy.max_pings_without_data, 3); - grpc_transport_destroy(&t->base); -} - -TEST_F(ConfigurationTest, ServerKeepaliveDefaults) { - ExecCtx exec_ctx; - grpc_chttp2_transport* t = reinterpret_cast( - grpc_create_chttp2_transport(args_, mock_endpoint_, /*is_client=*/false)); - EXPECT_EQ(t->keepalive_time, Duration::Hours(2)); - EXPECT_EQ(t->keepalive_timeout, Duration::Seconds(20)); - EXPECT_EQ(t->keepalive_permit_without_calls, false); - EXPECT_EQ(t->ping_policy.max_pings_without_data, 2); - EXPECT_EQ(t->ping_policy.min_recv_ping_interval_without_data, - Duration::Minutes(5)); - EXPECT_EQ(t->ping_policy.max_ping_strikes, 2); - grpc_transport_destroy(&t->base); -} - -TEST_F(ConfigurationTest, ServerKeepaliveExplicitArgs) { - ExecCtx exec_ctx; - args_ = args_.Set(GRPC_ARG_KEEPALIVE_TIME_MS, 20000); - args_ = args_.Set(GRPC_ARG_KEEPALIVE_TIMEOUT_MS, 10000); - args_ = args_.Set(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS, true); - args_ = args_.Set(GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, 3); - args_ = - args_.Set(GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS, 20000); - args_ = args_.Set(GRPC_ARG_HTTP2_MAX_PING_STRIKES, 0); - grpc_chttp2_transport* t = reinterpret_cast( - grpc_create_chttp2_transport(args_, mock_endpoint_, /*is_client=*/false)); - EXPECT_EQ(t->keepalive_time, Duration::Seconds(20)); - EXPECT_EQ(t->keepalive_timeout, Duration::Seconds(10)); - EXPECT_EQ(t->keepalive_permit_without_calls, true); - EXPECT_EQ(t->ping_policy.max_pings_without_data, 3); - EXPECT_EQ(t->ping_policy.min_recv_ping_interval_without_data, - Duration::Seconds(20)); - EXPECT_EQ(t->ping_policy.max_ping_strikes, 0); - grpc_transport_destroy(&t->base); -} - -// This test modifies the defaults of the client side settings, so it would -// affect any test that is run after this. -// TODO(yashykt): If adding more client side tests after this, add a reset to -// defaults function. -TEST_F(ConfigurationTest, ModifyClientDefaults) { - ExecCtx exec_ctx; - // Note that we are creating a new args object to override the defaults. - ChannelArgs args = args_.Set(GRPC_ARG_KEEPALIVE_TIME_MS, 20000); - args = args.Set(GRPC_ARG_KEEPALIVE_TIMEOUT_MS, 10000); - args = args.Set(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS, true); - args = args.Set(GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, 3); - grpc_chttp2_config_default_keepalive_args(args, /*is_client=*/true); - // Note that we are using the original args_ object for creating the transport - // which does not override the defaults. - grpc_chttp2_transport* t = reinterpret_cast( - grpc_create_chttp2_transport(args_, mock_endpoint_, /*is_client=*/true)); - EXPECT_EQ(t->keepalive_time, Duration::Seconds(20)); - EXPECT_EQ(t->keepalive_timeout, Duration::Seconds(10)); - EXPECT_EQ(t->keepalive_permit_without_calls, true); - EXPECT_EQ(t->ping_policy.max_pings_without_data, 3); - grpc_transport_destroy(&t->base); -} - -// This test modifies the defaults of the client side settings, so it would -// affect any test that is run after this. -// TODO(yashykt): If adding more server side tests after this, add a reset to -// defaults function. -TEST_F(ConfigurationTest, ModifyServerDefaults) { - ExecCtx exec_ctx; - // Note that we are creating a new args object to override the defaults. - ChannelArgs args = args_.Set(GRPC_ARG_KEEPALIVE_TIME_MS, 20000); - args = args.Set(GRPC_ARG_KEEPALIVE_TIMEOUT_MS, 10000); - args = args.Set(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS, true); - args = args.Set(GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, 3); - args = args.Set(GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS, 20000); - args = args.Set(GRPC_ARG_HTTP2_MAX_PING_STRIKES, 0); - grpc_chttp2_config_default_keepalive_args(args, /*is_client=*/false); - // Note that we are using the original args_ object for creating the transport - // which does not override the defaults. - grpc_chttp2_transport* t = reinterpret_cast( - grpc_create_chttp2_transport(args_, mock_endpoint_, /*is_client=*/false)); - EXPECT_EQ(t->keepalive_time, Duration::Seconds(20)); - EXPECT_EQ(t->keepalive_timeout, Duration::Seconds(10)); - EXPECT_EQ(t->keepalive_permit_without_calls, true); - EXPECT_EQ(t->ping_policy.max_pings_without_data, 3); - EXPECT_EQ(t->ping_policy.min_recv_ping_interval_without_data, - Duration::Seconds(20)); - EXPECT_EQ(t->ping_policy.max_ping_strikes, 0); - grpc_transport_destroy(&t->base); -} - -} // namespace -} // namespace grpc_core - -int main(int argc, char** argv) { - ::testing::InitGoogleTest(&argc, argv); - grpc::testing::TestEnvironment env(&argc, argv); - grpc_init(); - auto ret = RUN_ALL_TESTS(); - grpc_shutdown(); - return ret; -} diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 6c98574c118..1ca3b07110b 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -6311,30 +6311,6 @@ ], "uses_polling": true }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": true, - "language": "c++", - "name": "ping_configuration_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": false - }, { "args": [], "benchmark": false,