[experiments] Remove flow_control_fixes experiment (#33230)

We defaulted this on 5 months ago, and it seems to be working... let's
remove the experiment bit!

---------

Co-authored-by: ctiller <ctiller@users.noreply.github.com>

bazel/experiments.bzl

@@ -61,9 +61,6 @@ EXPERIMENTS = {
         ],
     },
     "on": {
-        "flow_control_test": [
-            "flow_control_fixes",
-        ],
     },
     "opt": {
     },

src/core/ext/transport/chttp2/transport/flow_control.cc

@@ -21,7 +21,6 @@
 #include "src/core/ext/transport/chttp2/transport/flow_control.h"
 
 #include <inttypes.h>
-#include <limits.h>
 
 #include <algorithm>
 #include <cmath>
@@ -278,7 +277,6 @@ void TransportFlowControl::UpdateSetting(
     uint32_t new_desired_value, FlowControlAction* action,
     FlowControlAction& (FlowControlAction::*set)(FlowControlAction::Urgency,
                                                  uint32_t)) {
-  if (IsFlowControlFixesEnabled()) {
   new_desired_value =
       Clamp(new_desired_value, grpc_chttp2_settings_parameters[id].min_value,
             grpc_chttp2_settings_parameters[id].max_value);
@@ -300,23 +298,12 @@ void TransportFlowControl::UpdateSetting(
     *desired_value = new_desired_value;
     (action->*set)(urgency, *desired_value);
   }
-  } else {
-    int64_t delta = new_desired_value - *desired_value;
-    // TODO(ncteisen): tune this
-    if (delta != 0 &&
-        (delta <= -*desired_value / 5 || delta >= *desired_value / 5)) {
-      *desired_value = new_desired_value;
-      (action->*set)(FlowControlAction::Urgency::QUEUE_UPDATE,
-                     static_cast<uint32_t>(*desired_value));
-    }
-  }
 }
 
 FlowControlAction TransportFlowControl::SetAckedInitialWindow(uint32_t value) {
   acked_init_window_ = value;
   FlowControlAction action;
-  if (IsFlowControlFixesEnabled() &&
+  if (acked_init_window_ != target_initial_window_size_) {
-      acked_init_window_ != target_initial_window_size_) {
     FlowControlAction::Urgency urgency =
         FlowControlAction::Urgency::QUEUE_UPDATE;
     if (acked_init_window_ == 0 || target_initial_window_size_ == 0) {
@@ -330,7 +317,6 @@ FlowControlAction TransportFlowControl::SetAckedInitialWindow(uint32_t value) {
 FlowControlAction TransportFlowControl::PeriodicUpdate() {
   FlowControlAction action;
   if (enable_bdp_probe_) {
-    if (IsFlowControlFixesEnabled()) {
     // get bdp estimate and update initial_window accordingly.
     // target might change based on how much memory pressure we are under
     // TODO(ncteisen): experiment with setting target to be huge under low
@@ -356,42 +342,6 @@ FlowControlAction TransportFlowControl::PeriodicUpdate() {
     UpdateSetting(GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE, &target_frame_size_,
                   target, &action,
                   &FlowControlAction::set_send_max_frame_size_update);
-    } else {
-      // get bdp estimate and update initial_window accordingly.
-      // target might change based on how much memory pressure we are under
-      // TODO(ncteisen): experiment with setting target to be huge under low
-      // memory pressure.
-      double target = IsMemoryPressureControllerEnabled()
-                          ? TargetInitialWindowSizeBasedOnMemoryPressureAndBdp()
-                          : pow(2, SmoothLogBdp(TargetLogBdp()));
-      if (g_test_only_transport_target_window_estimates_mocker != nullptr) {
-        // Hook for simulating unusual flow control situations in tests.
-        target = g_test_only_transport_target_window_estimates_mocker
-                     ->ComputeNextTargetInitialWindowSizeFromPeriodicUpdate(
-                         target_initial_window_size_ /* current target */);
-      }
-      // Though initial window 'could' drop to 0, we keep the floor at
-      // kMinInitialWindowSize
-      UpdateSetting(GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE,
-                    &target_initial_window_size_,
-                    static_cast<int32_t>(Clamp(
-                        target, static_cast<double>(kMinInitialWindowSize),
-                        static_cast<double>(kMaxInitialWindowSize))),
-                    &action,
-                    &FlowControlAction::set_send_initial_window_update);
-      // get bandwidth estimate and update max_frame accordingly.
-      double bw_dbl = bdp_estimator_.EstimateBandwidth();
-      // we target the max of BDP or bandwidth in microseconds.
-      UpdateSetting(
-          GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE, &target_frame_size_,
-          static_cast<int32_t>(
-              Clamp(std::max(static_cast<int32_t>(Clamp(
-                                 bw_dbl, 0.0, static_cast<double>(INT_MAX))) /
-                                 1000,
-                             static_cast<int32_t>(target_initial_window_size_)),
-                    16384, 16777215)),
-          &action, &FlowControlAction::set_send_max_frame_size_update);
-    }
 
     if (IsTcpFrameSizeTuningEnabled()) {
       // Advertise PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE to peer. By advertising

src/core/lib/experiments/experiments.cc

@@ -31,8 +31,6 @@ 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 =
     "New memory pressure controller";
 const char* const description_unconstrained_max_quota_buffer_size =
@@ -75,7 +73,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
     {"tcp_rcv_lowat", description_tcp_rcv_lowat, false, true},
     {"peer_state_based_framing", description_peer_state_based_framing, false,
      true},
-    {"flow_control_fixes", description_flow_control_fixes, true, true},
     {"memory_pressure_controller", description_memory_pressure_controller,
      false, true},
     {"unconstrained_max_quota_buffer_size",

src/core/lib/experiments/experiments.h

@@ -60,8 +60,6 @@ namespace grpc_core {
 inline bool IsTcpFrameSizeTuningEnabled() { return false; }
 inline bool IsTcpRcvLowatEnabled() { return false; }
 inline bool IsPeerStateBasedFramingEnabled() { return false; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_FLOW_CONTROL_FIXES
-inline bool IsFlowControlFixesEnabled() { return true; }
 inline bool IsMemoryPressureControllerEnabled() { return false; }
 inline bool IsUnconstrainedMaxQuotaBufferSizeEnabled() { return false; }
 inline bool IsEventEngineClientEnabled() { return false; }
@@ -86,52 +84,48 @@ inline bool IsTcpFrameSizeTuningEnabled() { return IsExperimentEnabled(0); }
 inline bool IsTcpRcvLowatEnabled() { return IsExperimentEnabled(1); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_PEER_STATE_BASED_FRAMING
 inline bool IsPeerStateBasedFramingEnabled() { return IsExperimentEnabled(2); }
-#define GRPC_EXPERIMENT_IS_INCLUDED_FLOW_CONTROL_FIXES
-inline bool IsFlowControlFixesEnabled() { return IsExperimentEnabled(3); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_MEMORY_PRESSURE_CONTROLLER
 inline bool IsMemoryPressureControllerEnabled() {
-  return IsExperimentEnabled(4);
+  return IsExperimentEnabled(3);
 }
 #define GRPC_EXPERIMENT_IS_INCLUDED_UNCONSTRAINED_MAX_QUOTA_BUFFER_SIZE
 inline bool IsUnconstrainedMaxQuotaBufferSizeEnabled() {
-  return IsExperimentEnabled(5);
+  return IsExperimentEnabled(4);
 }
 #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_CLIENT
-inline bool IsEventEngineClientEnabled() { return IsExperimentEnabled(6); }
+inline bool IsEventEngineClientEnabled() { return IsExperimentEnabled(5); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_MONITORING_EXPERIMENT
-inline bool IsMonitoringExperimentEnabled() { return IsExperimentEnabled(7); }
+inline bool IsMonitoringExperimentEnabled() { return IsExperimentEnabled(6); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_PROMISE_BASED_CLIENT_CALL
-inline bool IsPromiseBasedClientCallEnabled() { return IsExperimentEnabled(8); }
+inline bool IsPromiseBasedClientCallEnabled() { return IsExperimentEnabled(7); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_FREE_LARGE_ALLOCATOR
-inline bool IsFreeLargeAllocatorEnabled() { return IsExperimentEnabled(9); }
+inline bool IsFreeLargeAllocatorEnabled() { return IsExperimentEnabled(8); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_PROMISE_BASED_SERVER_CALL
-inline bool IsPromiseBasedServerCallEnabled() {
+inline bool IsPromiseBasedServerCallEnabled() { return IsExperimentEnabled(9); }
-  return IsExperimentEnabled(10);
-}
 #define GRPC_EXPERIMENT_IS_INCLUDED_TRANSPORT_SUPPLIES_CLIENT_LATENCY
 inline bool IsTransportSuppliesClientLatencyEnabled() {
-  return IsExperimentEnabled(11);
+  return IsExperimentEnabled(10);
 }
 #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_LISTENER
-inline bool IsEventEngineListenerEnabled() { return IsExperimentEnabled(12); }
+inline bool IsEventEngineListenerEnabled() { return IsExperimentEnabled(11); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_SCHEDULE_CANCELLATION_OVER_WRITE
 inline bool IsScheduleCancellationOverWriteEnabled() {
-  return IsExperimentEnabled(13);
+  return IsExperimentEnabled(12);
 }
 #define GRPC_EXPERIMENT_IS_INCLUDED_TRACE_RECORD_CALLOPS
-inline bool IsTraceRecordCallopsEnabled() { return IsExperimentEnabled(14); }
+inline bool IsTraceRecordCallopsEnabled() { return IsExperimentEnabled(13); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_DNS
-inline bool IsEventEngineDnsEnabled() { return IsExperimentEnabled(15); }
+inline bool IsEventEngineDnsEnabled() { return IsExperimentEnabled(14); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_WORK_STEALING
-inline bool IsWorkStealingEnabled() { return IsExperimentEnabled(16); }
+inline bool IsWorkStealingEnabled() { return IsExperimentEnabled(15); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_CLIENT_PRIVACY
-inline bool IsClientPrivacyEnabled() { return IsExperimentEnabled(17); }
+inline bool IsClientPrivacyEnabled() { return IsExperimentEnabled(16); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_CANARY_CLIENT_PRIVACY
-inline bool IsCanaryClientPrivacyEnabled() { return IsExperimentEnabled(18); }
+inline bool IsCanaryClientPrivacyEnabled() { return IsExperimentEnabled(17); }
 #define GRPC_EXPERIMENT_IS_INCLUDED_SERVER_PRIVACY
-inline bool IsServerPrivacyEnabled() { return IsExperimentEnabled(19); }
+inline bool IsServerPrivacyEnabled() { return IsExperimentEnabled(18); }
 
-constexpr const size_t kNumExperiments = 20;
+constexpr const size_t kNumExperiments = 19;
 extern const ExperimentMetadata g_experiment_metadata[kNumExperiments];
 
 #endif

src/core/lib/experiments/experiments.yaml

@@ -64,12 +64,6 @@
   expiry: 2023/06/01
   owner: vigneshbabu@google.com
   test_tags: ["flow_control_test"]
-- name: flow_control_fixes
-  description: Various fixes for flow control, max frame size setting.
-  default: true
-  expiry: 2023/06/01
-  owner: ctiller@google.com
-  test_tags: ["flow_control_test"]
 - name: memory_pressure_controller
   description: New memory pressure controller
   default: false