[keepalive] Allow server side keepalive_permit_without_calls setting to be overridden (#33917)

Need the ability to override server-side keepalive permit without calls
default without affecting client-side settings.
<!--

If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the
appropriate
lang label.

-->
pull/33933/head
Yash Tibrewal 1 year ago committed by GitHub
parent 9de738d9e5
commit 010a59b7fd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 10
      src/core/ext/transport/chttp2/transport/chttp2_transport.cc
  2. 18
      src/core/lib/experiments/experiments.cc
  3. 7
      src/core/lib/experiments/experiments.h
  4. 8
      src/core/lib/experiments/experiments.yaml
  5. 2
      src/core/lib/experiments/rollouts.yaml
  6. 1
      test/core/transport/chttp2/ping_configuration_test.cc

@ -398,16 +398,18 @@ static void read_channel_args(grpc_chttp2_transport* t,
channel_args.GetDurationFromIntMillis(GRPC_ARG_KEEPALIVE_TIMEOUT_MS) channel_args.GetDurationFromIntMillis(GRPC_ARG_KEEPALIVE_TIMEOUT_MS)
.value_or(t->is_client ? g_default_client_keepalive_timeout .value_or(t->is_client ? g_default_client_keepalive_timeout
: g_default_server_keepalive_timeout)); : g_default_server_keepalive_timeout));
if (grpc_core::IsKeepaliveFixEnabled()) { if (t->is_client) {
t->keepalive_permit_without_calls = t->keepalive_permit_without_calls =
channel_args.GetBool(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS) channel_args.GetBool(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS)
.value_or(t->is_client .value_or(grpc_core::IsKeepaliveFixEnabled()
? g_default_client_keepalive_permit_without_calls ? g_default_client_keepalive_permit_without_calls
: g_default_server_keepalive_permit_without_calls); : false);
} else { } else {
t->keepalive_permit_without_calls = t->keepalive_permit_without_calls =
channel_args.GetBool(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS) channel_args.GetBool(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS)
.value_or(false); .value_or(grpc_core::IsKeepaliveServerFixEnabled()
? g_default_server_keepalive_permit_without_calls
: false);
} }
// Only send the prefered rx frame size http2 setting if we are instructed // Only send the prefered rx frame size http2 setting if we are instructed

@ -97,6 +97,10 @@ const char* const description_keepalive_fix =
"Allows overriding keepalive_permit_without_calls. Refer " "Allows overriding keepalive_permit_without_calls. Refer "
"https://github.com/grpc/grpc/pull/33428 for more information."; "https://github.com/grpc/grpc/pull/33428 for more information.";
const char* const additional_constraints_keepalive_fix = "{}"; const char* const additional_constraints_keepalive_fix = "{}";
const char* const description_keepalive_server_fix =
"Allows overriding keepalive_permit_without_calls for servers. Refer "
"https://github.com/grpc/grpc/pull/33917 for more information.";
const char* const additional_constraints_keepalive_server_fix = "{}";
#ifdef NDEBUG #ifdef NDEBUG
const bool kDefaultForDebugOnly = false; const bool kDefaultForDebugOnly = false;
#else #else
@ -152,6 +156,8 @@ const ExperimentMetadata g_experiment_metadata[] = {
additional_constraints_unique_metadata_strings, false, true}, additional_constraints_unique_metadata_strings, false, true},
{"keepalive_fix", description_keepalive_fix, {"keepalive_fix", description_keepalive_fix,
additional_constraints_keepalive_fix, false, false}, additional_constraints_keepalive_fix, false, false},
{"keepalive_server_fix", description_keepalive_server_fix,
additional_constraints_keepalive_server_fix, false, false},
}; };
} // namespace grpc_core } // namespace grpc_core
@ -233,6 +239,10 @@ const char* const description_keepalive_fix =
"Allows overriding keepalive_permit_without_calls. Refer " "Allows overriding keepalive_permit_without_calls. Refer "
"https://github.com/grpc/grpc/pull/33428 for more information."; "https://github.com/grpc/grpc/pull/33428 for more information.";
const char* const additional_constraints_keepalive_fix = "{}"; const char* const additional_constraints_keepalive_fix = "{}";
const char* const description_keepalive_server_fix =
"Allows overriding keepalive_permit_without_calls for servers. Refer "
"https://github.com/grpc/grpc/pull/33917 for more information.";
const char* const additional_constraints_keepalive_server_fix = "{}";
#ifdef NDEBUG #ifdef NDEBUG
const bool kDefaultForDebugOnly = false; const bool kDefaultForDebugOnly = false;
#else #else
@ -288,6 +298,8 @@ const ExperimentMetadata g_experiment_metadata[] = {
additional_constraints_unique_metadata_strings, false, true}, additional_constraints_unique_metadata_strings, false, true},
{"keepalive_fix", description_keepalive_fix, {"keepalive_fix", description_keepalive_fix,
additional_constraints_keepalive_fix, false, false}, additional_constraints_keepalive_fix, false, false},
{"keepalive_server_fix", description_keepalive_server_fix,
additional_constraints_keepalive_server_fix, false, false},
}; };
} // namespace grpc_core } // namespace grpc_core
@ -369,6 +381,10 @@ const char* const description_keepalive_fix =
"Allows overriding keepalive_permit_without_calls. Refer " "Allows overriding keepalive_permit_without_calls. Refer "
"https://github.com/grpc/grpc/pull/33428 for more information."; "https://github.com/grpc/grpc/pull/33428 for more information.";
const char* const additional_constraints_keepalive_fix = "{}"; const char* const additional_constraints_keepalive_fix = "{}";
const char* const description_keepalive_server_fix =
"Allows overriding keepalive_permit_without_calls for servers. Refer "
"https://github.com/grpc/grpc/pull/33917 for more information.";
const char* const additional_constraints_keepalive_server_fix = "{}";
#ifdef NDEBUG #ifdef NDEBUG
const bool kDefaultForDebugOnly = false; const bool kDefaultForDebugOnly = false;
#else #else
@ -424,6 +440,8 @@ const ExperimentMetadata g_experiment_metadata[] = {
additional_constraints_unique_metadata_strings, false, true}, additional_constraints_unique_metadata_strings, false, true},
{"keepalive_fix", description_keepalive_fix, {"keepalive_fix", description_keepalive_fix,
additional_constraints_keepalive_fix, false, false}, additional_constraints_keepalive_fix, false, false},
{"keepalive_server_fix", description_keepalive_server_fix,
additional_constraints_keepalive_server_fix, false, false},
}; };
} // namespace grpc_core } // namespace grpc_core

@ -90,6 +90,7 @@ inline bool IsCanaryClientPrivacyEnabled() { return false; }
inline bool IsServerPrivacyEnabled() { return false; } inline bool IsServerPrivacyEnabled() { return false; }
inline bool IsUniqueMetadataStringsEnabled() { return false; } inline bool IsUniqueMetadataStringsEnabled() { return false; }
inline bool IsKeepaliveFixEnabled() { return false; } inline bool IsKeepaliveFixEnabled() { return false; }
inline bool IsKeepaliveServerFixEnabled() { return false; }
#elif defined(GPR_WINDOWS) #elif defined(GPR_WINDOWS)
inline bool IsTcpFrameSizeTuningEnabled() { return false; } inline bool IsTcpFrameSizeTuningEnabled() { return false; }
@ -123,6 +124,7 @@ inline bool IsCanaryClientPrivacyEnabled() { return false; }
inline bool IsServerPrivacyEnabled() { return false; } inline bool IsServerPrivacyEnabled() { return false; }
inline bool IsUniqueMetadataStringsEnabled() { return false; } inline bool IsUniqueMetadataStringsEnabled() { return false; }
inline bool IsKeepaliveFixEnabled() { return false; } inline bool IsKeepaliveFixEnabled() { return false; }
inline bool IsKeepaliveServerFixEnabled() { return false; }
#else #else
inline bool IsTcpFrameSizeTuningEnabled() { return false; } inline bool IsTcpFrameSizeTuningEnabled() { return false; }
@ -156,6 +158,7 @@ inline bool IsCanaryClientPrivacyEnabled() { return false; }
inline bool IsServerPrivacyEnabled() { return false; } inline bool IsServerPrivacyEnabled() { return false; }
inline bool IsUniqueMetadataStringsEnabled() { return false; } inline bool IsUniqueMetadataStringsEnabled() { return false; }
inline bool IsKeepaliveFixEnabled() { return false; } inline bool IsKeepaliveFixEnabled() { return false; }
inline bool IsKeepaliveServerFixEnabled() { return false; }
#endif #endif
#else #else
@ -209,8 +212,10 @@ inline bool IsServerPrivacyEnabled() { return IsExperimentEnabled(18); }
inline bool IsUniqueMetadataStringsEnabled() { return IsExperimentEnabled(19); } inline bool IsUniqueMetadataStringsEnabled() { return IsExperimentEnabled(19); }
#define GRPC_EXPERIMENT_IS_INCLUDED_KEEPALIVE_FIX #define GRPC_EXPERIMENT_IS_INCLUDED_KEEPALIVE_FIX
inline bool IsKeepaliveFixEnabled() { return IsExperimentEnabled(20); } inline bool IsKeepaliveFixEnabled() { return IsExperimentEnabled(20); }
#define GRPC_EXPERIMENT_IS_INCLUDED_KEEPALIVE_SERVER_FIX
inline bool IsKeepaliveServerFixEnabled() { return IsExperimentEnabled(21); }
constexpr const size_t kNumExperiments = 21; constexpr const size_t kNumExperiments = 22;
extern const ExperimentMetadata g_experiment_metadata[kNumExperiments]; extern const ExperimentMetadata g_experiment_metadata[kNumExperiments];
#endif #endif

@ -166,3 +166,11 @@
owner: yashkt@google.com owner: yashkt@google.com
test_tags: [] test_tags: []
allow_in_fuzzing_config: false allow_in_fuzzing_config: false
- name: keepalive_server_fix
description:
Allows overriding keepalive_permit_without_calls for servers.
Refer https://github.com/grpc/grpc/pull/33917 for more information.
expiry: 2023/09/07
owner: yashkt@google.com
test_tags: []
allow_in_fuzzing_config: false

@ -90,3 +90,5 @@
default: false default: false
- name: keepalive_fix - name: keepalive_fix
default: false default: false
- name: keepalive_server_fix
default: false

@ -175,6 +175,7 @@ int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv); ::testing::InitGoogleTest(&argc, argv);
grpc::testing::TestEnvironment env(&argc, argv); grpc::testing::TestEnvironment env(&argc, argv);
grpc_core::ForceEnableExperiment("keepalive_fix", true); grpc_core::ForceEnableExperiment("keepalive_fix", true);
grpc_core::ForceEnableExperiment("keepalive_server_fix", true);
grpc_init(); grpc_init();
auto ret = RUN_ALL_TESTS(); auto ret = RUN_ALL_TESTS();
grpc_shutdown(); grpc_shutdown();

Loading…
Cancel
Save