From f9b866ff8bc0daa415db90ac98faa9f6ba04ebf8 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 8 Sep 2022 08:54:24 -0700 Subject: [PATCH] [chttp2] Use experiments framework for peer_state_based_framing (#30853) * [chttp2] Use experiments framework for peer_state_based_framing * Automated change: Fix sanity tests Co-authored-by: ctiller --- BUILD | 1 + bazel/experiments.bzl | 1 + .../transport/chttp2/transport/chttp2_transport.cc | 14 +++----------- src/core/lib/experiments/experiments.cc | 5 +++++ src/core/lib/experiments/experiments.h | 11 ++++++----- src/core/lib/experiments/experiments.yaml | 9 +++++++++ 6 files changed, 25 insertions(+), 16 deletions(-) diff --git a/BUILD b/BUILD index c490f216bdb..76558731c79 100644 --- a/BUILD +++ b/BUILD @@ -6746,6 +6746,7 @@ grpc_cc_library( "bitset", "chttp2_flow_control", "debug_location", + "experiments", "gpr", "grpc_base", "grpc_codegen", diff --git a/bazel/experiments.bzl b/bazel/experiments.bzl index 609d72f0779..7f7520d1c22 100644 --- a/bazel/experiments.bzl +++ b/bazel/experiments.bzl @@ -24,6 +24,7 @@ EXPERIMENTS = { ], "flow_control_test": [ "flow_control_fixes", + "peer_state_based_framing", "tcp_frame_size_tuning", "tcp_rcv_lowat", "tcp_read_chunks", diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index a32b558dfac..72f418b3d7a 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -60,10 +60,10 @@ #include "src/core/ext/transport/chttp2/transport/varint.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/debug/stats_data.h" +#include "src/core/lib/experiments/experiments.h" #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/bitset.h" #include "src/core/lib/gprpp/debug_location.h" -#include "src/core/lib/gprpp/global_config_env.h" #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/status_helper.h" #include "src/core/lib/gprpp/time.h" @@ -91,12 +91,6 @@ #include "src/core/lib/transport/transport.h" #include "src/core/lib/transport/transport_impl.h" -GPR_GLOBAL_CONFIG_DEFINE_BOOL( - grpc_experimental_enable_peer_state_based_framing, false, - "If set, the max sizes of frames sent to lower layers is controlled based " - "on the peer's memory pressure which is reflected in its max http2 frame " - "size."); - #define DEFAULT_CONNECTION_WINDOW_TARGET (1024 * 1024) #define MAX_WINDOW 0x7fffffffu #define MAX_WRITE_BUFFER_SIZE (64 * 1024 * 1024) @@ -878,18 +872,16 @@ static void write_action_begin_locked(void* gt, } static void write_action(void* gt, grpc_error_handle /*error*/) { - static bool kEnablePeerStateBasedFraming = - GPR_GLOBAL_CONFIG_GET(grpc_experimental_enable_peer_state_based_framing); grpc_chttp2_transport* t = static_cast(gt); void* cl = t->cl; t->cl = nullptr; - // If grpc_experimental_enable_peer_state_based_framing is set to true, + // If the peer_state_based_framing experiment is set to true, // choose max_frame_size as 2 * max http2 frame size of peer. If peer is under // high memory pressure, then it would advertise a smaller max http2 frame // size. With this logic, the sender would automatically reduce the sending // frame size as well. int max_frame_size = - kEnablePeerStateBasedFraming + grpc_core::IsPeerStateBasedFramingEnabled() ? 2 * t->settings[GRPC_PEER_SETTINGS] [GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE] : INT_MAX; diff --git a/src/core/lib/experiments/experiments.cc b/src/core/lib/experiments/experiments.cc index 523a57acd95..683b4880d1e 100644 --- a/src/core/lib/experiments/experiments.cc +++ b/src/core/lib/experiments/experiments.cc @@ -29,6 +29,10 @@ const char* const description_tcp_read_chunks = "malloc to recycle arbitrary large blocks."; const char* const description_tcp_rcv_lowat = "Use SO_RCVLOWAT to avoid wakeups on the read path."; +const char* const description_peer_state_based_framing = + "If set, the max sizes of frames sent to lower layers is controlled based " + "on the peer's memory pressure which is reflected in its max http2 frame " + "size."; const char* const description_flow_control_fixes = "Various fixes for flow control, max frame size setting."; const char* const description_memory_pressure_controller = @@ -45,6 +49,7 @@ const ExperimentMetadata g_experiment_metadata[] = { {"tcp_frame_size_tuning", description_tcp_frame_size_tuning, false}, {"tcp_read_chunks", description_tcp_read_chunks, false}, {"tcp_rcv_lowat", description_tcp_rcv_lowat, false}, + {"peer_state_based_framing", description_peer_state_based_framing, false}, {"flow_control_fixes", description_flow_control_fixes, false}, {"memory_pressure_controller", description_memory_pressure_controller, false}, diff --git a/src/core/lib/experiments/experiments.h b/src/core/lib/experiments/experiments.h index e51061182e3..7d812c0fa4b 100644 --- a/src/core/lib/experiments/experiments.h +++ b/src/core/lib/experiments/experiments.h @@ -28,15 +28,16 @@ namespace grpc_core { inline bool IsTcpFrameSizeTuningEnabled() { return IsExperimentEnabled(0); } inline bool IsTcpReadChunksEnabled() { return IsExperimentEnabled(1); } inline bool IsTcpRcvLowatEnabled() { return IsExperimentEnabled(2); } -inline bool IsFlowControlFixesEnabled() { return IsExperimentEnabled(3); } +inline bool IsPeerStateBasedFramingEnabled() { return IsExperimentEnabled(3); } +inline bool IsFlowControlFixesEnabled() { return IsExperimentEnabled(4); } inline bool IsMemoryPressureControllerEnabled() { - return IsExperimentEnabled(4); + return IsExperimentEnabled(5); } inline bool IsPeriodicResourceQuotaReclamationEnabled() { - return IsExperimentEnabled(5); + return IsExperimentEnabled(6); } inline bool IsUnconstrainedMaxQuotaBufferSizeEnabled() { - return IsExperimentEnabled(6); + return IsExperimentEnabled(7); } struct ExperimentMetadata { @@ -45,7 +46,7 @@ struct ExperimentMetadata { bool default_value; }; -constexpr const size_t kNumExperiments = 7; +constexpr const size_t kNumExperiments = 8; extern const ExperimentMetadata g_experiment_metadata[kNumExperiments]; } // namespace grpc_core diff --git a/src/core/lib/experiments/experiments.yaml b/src/core/lib/experiments/experiments.yaml index d5351abf9ac..5d70039caae 100644 --- a/src/core/lib/experiments/experiments.yaml +++ b/src/core/lib/experiments/experiments.yaml @@ -51,6 +51,15 @@ expiry: 2022/10/01 owner: ctiller@google.com test_tags: ["endpoint_test", "flow_control_test"] +- name: peer_state_based_framing + description: + If set, the max sizes of frames sent to lower layers is controlled based + on the peer's memory pressure which is reflected in its max http2 frame + size. + default: false + expiry: 2022/11/01 + owner: vigneshbabu@google.com + test_tags: ["flow_control_test"] - name: flow_control_fixes description: Various fixes for flow control, max frame size setting.