diff --git a/CMakeLists.txt b/CMakeLists.txt index 939249812d2..17d1a7410a8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1194,7 +1194,6 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx percent_encoding_test) 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_abuse_policy_test) add_dependencies(buildtests_cxx ping_callbacks_test) add_dependencies(buildtests_cxx ping_configuration_test) @@ -2505,7 +2504,6 @@ add_library(grpc src/core/lib/transport/http_connect_handshaker.cc src/core/lib/transport/metadata_batch.cc src/core/lib/transport/parsed_metadata.cc - src/core/lib/transport/pid_controller.cc src/core/lib/transport/status_conversion.cc src/core/lib/transport/tcp_connect_handshaker.cc src/core/lib/transport/timeout_encoding.cc @@ -3198,7 +3196,6 @@ add_library(grpc_unsecure src/core/lib/transport/http_connect_handshaker.cc src/core/lib/transport/metadata_batch.cc src/core/lib/transport/parsed_metadata.cc - src/core/lib/transport/pid_controller.cc src/core/lib/transport/status_conversion.cc src/core/lib/transport/tcp_connect_handshaker.cc src/core/lib/transport/timeout_encoding.cc @@ -12230,7 +12227,6 @@ add_executable(flow_control_test src/core/lib/slice/slice_refcount.cc src/core/lib/slice/slice_string_helpers.cc src/core/lib/transport/bdp_estimator.cc - src/core/lib/transport/pid_controller.cc test/core/transport/chttp2/flow_control_test.cc third_party/upb/upb/hash/common.c third_party/upb/upb/message/accessors.c @@ -17805,49 +17801,6 @@ target_link_libraries(pick_first_test ) -endif() -if(gRPC_BUILD_TESTS) - -add_executable(pid_controller_test - test/core/transport/pid_controller_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/tracer_util.cc -) -target_compile_features(pid_controller_test PUBLIC cxx_std_14) -target_include_directories(pid_controller_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(pid_controller_test - ${_gRPC_ALLTARGETS_LIBRARIES} - gtest - grpc_test_util -) - - endif() if(gRPC_BUILD_TESTS) diff --git a/Makefile b/Makefile index a1f5f107702..0071fee977c 100644 --- a/Makefile +++ b/Makefile @@ -1705,7 +1705,6 @@ LIBGRPC_SRC = \ src/core/lib/transport/http_connect_handshaker.cc \ src/core/lib/transport/metadata_batch.cc \ src/core/lib/transport/parsed_metadata.cc \ - src/core/lib/transport/pid_controller.cc \ src/core/lib/transport/status_conversion.cc \ src/core/lib/transport/tcp_connect_handshaker.cc \ src/core/lib/transport/timeout_encoding.cc \ @@ -2248,7 +2247,6 @@ LIBGRPC_UNSECURE_SRC = \ src/core/lib/transport/http_connect_handshaker.cc \ src/core/lib/transport/metadata_batch.cc \ src/core/lib/transport/parsed_metadata.cc \ - src/core/lib/transport/pid_controller.cc \ src/core/lib/transport/status_conversion.cc \ src/core/lib/transport/tcp_connect_handshaker.cc \ src/core/lib/transport/timeout_encoding.cc \ diff --git a/Package.swift b/Package.swift index 7212ca11453..1f8a60eb738 100644 --- a/Package.swift +++ b/Package.swift @@ -1898,8 +1898,6 @@ let package = Package( "src/core/lib/transport/metadata_compression_traits.h", "src/core/lib/transport/parsed_metadata.cc", "src/core/lib/transport/parsed_metadata.h", - "src/core/lib/transport/pid_controller.cc", - "src/core/lib/transport/pid_controller.h", "src/core/lib/transport/simple_slice_based_metadata.h", "src/core/lib/transport/status_conversion.cc", "src/core/lib/transport/status_conversion.h", diff --git a/bazel/experiments.bzl b/bazel/experiments.bzl index b54ea252ac2..a41430e9d7c 100644 --- a/bazel/experiments.bzl +++ b/bazel/experiments.bzl @@ -28,16 +28,13 @@ EXPERIMENT_ENABLES = { "http2_stats_fix": "http2_stats_fix", "keepalive_fix": "keepalive_fix", "keepalive_server_fix": "keepalive_server_fix", - "memory_pressure_controller": "memory_pressure_controller", "monitoring_experiment": "monitoring_experiment", "multiping": "multiping", - "overload_protection": "overload_protection", "peer_state_based_framing": "peer_state_based_framing", "pending_queue_cap": "pending_queue_cap", "pick_first_happy_eyeballs": "pick_first_happy_eyeballs", "promise_based_client_call": "promise_based_client_call", "promise_based_server_call": "promise_based_server_call", - "red_max_concurrent_streams": "red_max_concurrent_streams", "registered_method_lookup_in_transport": "registered_method_lookup_in_transport", "promise_based_inproc_transport": "promise_based_client_call,promise_based_inproc_transport,promise_based_server_call,registered_method_lookup_in_transport", "registered_methods_map": "registered_methods_map", @@ -88,7 +85,6 @@ EXPERIMENTS = { "flow_control_test": [ "multiping", "peer_state_based_framing", - "red_max_concurrent_streams", "rstpit", "tcp_frame_size_tuning", "tcp_rcv_lowat", @@ -104,7 +100,6 @@ EXPERIMENTS = { ], "resource_quota_test": [ "free_large_allocator", - "memory_pressure_controller", "unconstrained_max_quota_buffer_size", ], "xds_end2end_test": [ @@ -125,7 +120,6 @@ EXPERIMENTS = { "event_engine_listener", ], "flow_control_test": [ - "overload_protection", "write_size_cap", "write_size_policy", ], @@ -170,7 +164,6 @@ EXPERIMENTS = { "flow_control_test": [ "multiping", "peer_state_based_framing", - "red_max_concurrent_streams", "rstpit", "tcp_frame_size_tuning", "tcp_rcv_lowat", @@ -186,7 +179,6 @@ EXPERIMENTS = { ], "resource_quota_test": [ "free_large_allocator", - "memory_pressure_controller", "unconstrained_max_quota_buffer_size", ], "xds_end2end_test": [ @@ -201,7 +193,6 @@ EXPERIMENTS = { "wrr_delegate_to_pick_first", ], "flow_control_test": [ - "overload_protection", "write_size_cap", "write_size_policy", ], @@ -253,7 +244,6 @@ EXPERIMENTS = { "flow_control_test": [ "multiping", "peer_state_based_framing", - "red_max_concurrent_streams", "rstpit", "tcp_frame_size_tuning", "tcp_rcv_lowat", @@ -272,7 +262,6 @@ EXPERIMENTS = { ], "resource_quota_test": [ "free_large_allocator", - "memory_pressure_controller", "unconstrained_max_quota_buffer_size", ], "xds_end2end_test": [ @@ -293,7 +282,6 @@ EXPERIMENTS = { "event_engine_listener", ], "flow_control_test": [ - "overload_protection", "write_size_cap", "write_size_policy", ], diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 1849ce804a2..b1492ba7068 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -1175,7 +1175,6 @@ libs: - src/core/lib/transport/metadata_batch.h - src/core/lib/transport/metadata_compression_traits.h - src/core/lib/transport/parsed_metadata.h - - src/core/lib/transport/pid_controller.h - src/core/lib/transport/simple_slice_based_metadata.h - src/core/lib/transport/status_conversion.h - src/core/lib/transport/tcp_connect_handshaker.h @@ -1965,7 +1964,6 @@ libs: - src/core/lib/transport/http_connect_handshaker.cc - src/core/lib/transport/metadata_batch.cc - src/core/lib/transport/parsed_metadata.cc - - src/core/lib/transport/pid_controller.cc - src/core/lib/transport/status_conversion.cc - src/core/lib/transport/tcp_connect_handshaker.cc - src/core/lib/transport/timeout_encoding.cc @@ -2606,7 +2604,6 @@ libs: - src/core/lib/transport/metadata_batch.h - src/core/lib/transport/metadata_compression_traits.h - src/core/lib/transport/parsed_metadata.h - - src/core/lib/transport/pid_controller.h - src/core/lib/transport/simple_slice_based_metadata.h - src/core/lib/transport/status_conversion.h - src/core/lib/transport/tcp_connect_handshaker.h @@ -3016,7 +3013,6 @@ libs: - src/core/lib/transport/http_connect_handshaker.cc - src/core/lib/transport/metadata_batch.cc - src/core/lib/transport/parsed_metadata.cc - - src/core/lib/transport/pid_controller.cc - src/core/lib/transport/status_conversion.cc - src/core/lib/transport/tcp_connect_handshaker.cc - src/core/lib/transport/timeout_encoding.cc @@ -9283,7 +9279,6 @@ targets: - src/core/lib/slice/slice_string_helpers.h - src/core/lib/transport/bdp_estimator.h - src/core/lib/transport/http2_errors.h - - src/core/lib/transport/pid_controller.h - third_party/upb/upb/base/internal/log2.h - third_party/upb/upb/generated_code_support.h - third_party/upb/upb/hash/common.h @@ -9365,7 +9360,6 @@ targets: - src/core/lib/slice/slice_refcount.cc - src/core/lib/slice/slice_string_helpers.cc - src/core/lib/transport/bdp_estimator.cc - - src/core/lib/transport/pid_controller.cc - test/core/transport/chttp2/flow_control_test.cc - third_party/upb/upb/hash/common.c - third_party/upb/upb/message/accessors.c @@ -12712,38 +12706,6 @@ targets: - protobuf - grpc_test_util uses_polling: false -- name: pid_controller_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/tracer_util.h - src: - - test/core/transport/pid_controller_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/tracer_util.cc - deps: - - gtest - - grpc_test_util - name: ping_abuse_policy_test gtest: true build: test diff --git a/config.m4 b/config.m4 index c26543be07d..34b99719662 100644 --- a/config.m4 +++ b/config.m4 @@ -837,7 +837,6 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/transport/http_connect_handshaker.cc \ src/core/lib/transport/metadata_batch.cc \ src/core/lib/transport/parsed_metadata.cc \ - src/core/lib/transport/pid_controller.cc \ src/core/lib/transport/status_conversion.cc \ src/core/lib/transport/tcp_connect_handshaker.cc \ src/core/lib/transport/timeout_encoding.cc \ diff --git a/config.w32 b/config.w32 index f2ed1de805b..d7678fff5ac 100644 --- a/config.w32 +++ b/config.w32 @@ -802,7 +802,6 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\transport\\http_connect_handshaker.cc " + "src\\core\\lib\\transport\\metadata_batch.cc " + "src\\core\\lib\\transport\\parsed_metadata.cc " + - "src\\core\\lib\\transport\\pid_controller.cc " + "src\\core\\lib\\transport\\status_conversion.cc " + "src\\core\\lib\\transport\\tcp_connect_handshaker.cc " + "src\\core\\lib\\transport\\timeout_encoding.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 94b37911d90..ff49bb43910 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -1270,7 +1270,6 @@ Pod::Spec.new do |s| 'src/core/lib/transport/metadata_batch.h', 'src/core/lib/transport/metadata_compression_traits.h', 'src/core/lib/transport/parsed_metadata.h', - 'src/core/lib/transport/pid_controller.h', 'src/core/lib/transport/simple_slice_based_metadata.h', 'src/core/lib/transport/status_conversion.h', 'src/core/lib/transport/tcp_connect_handshaker.h', @@ -2512,7 +2511,6 @@ Pod::Spec.new do |s| 'src/core/lib/transport/metadata_batch.h', 'src/core/lib/transport/metadata_compression_traits.h', 'src/core/lib/transport/parsed_metadata.h', - 'src/core/lib/transport/pid_controller.h', 'src/core/lib/transport/simple_slice_based_metadata.h', 'src/core/lib/transport/status_conversion.h', 'src/core/lib/transport/tcp_connect_handshaker.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 2784969c409..bebce4fc733 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1997,8 +1997,6 @@ Pod::Spec.new do |s| 'src/core/lib/transport/metadata_compression_traits.h', 'src/core/lib/transport/parsed_metadata.cc', 'src/core/lib/transport/parsed_metadata.h', - 'src/core/lib/transport/pid_controller.cc', - 'src/core/lib/transport/pid_controller.h', 'src/core/lib/transport/simple_slice_based_metadata.h', 'src/core/lib/transport/status_conversion.cc', 'src/core/lib/transport/status_conversion.h', @@ -3279,7 +3277,6 @@ Pod::Spec.new do |s| 'src/core/lib/transport/metadata_batch.h', 'src/core/lib/transport/metadata_compression_traits.h', 'src/core/lib/transport/parsed_metadata.h', - 'src/core/lib/transport/pid_controller.h', 'src/core/lib/transport/simple_slice_based_metadata.h', 'src/core/lib/transport/status_conversion.h', 'src/core/lib/transport/tcp_connect_handshaker.h', diff --git a/grpc.gemspec b/grpc.gemspec index ecbb77b1bb1..4a2e017e484 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1900,8 +1900,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/transport/metadata_compression_traits.h ) s.files += %w( src/core/lib/transport/parsed_metadata.cc ) s.files += %w( src/core/lib/transport/parsed_metadata.h ) - s.files += %w( src/core/lib/transport/pid_controller.cc ) - s.files += %w( src/core/lib/transport/pid_controller.h ) s.files += %w( src/core/lib/transport/simple_slice_based_metadata.h ) s.files += %w( src/core/lib/transport/status_conversion.cc ) s.files += %w( src/core/lib/transport/status_conversion.h ) diff --git a/grpc.gyp b/grpc.gyp index f8ba94de815..3a5ba2d57d0 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -1024,7 +1024,6 @@ 'src/core/lib/transport/http_connect_handshaker.cc', 'src/core/lib/transport/metadata_batch.cc', 'src/core/lib/transport/parsed_metadata.cc', - 'src/core/lib/transport/pid_controller.cc', 'src/core/lib/transport/status_conversion.cc', 'src/core/lib/transport/tcp_connect_handshaker.cc', 'src/core/lib/transport/timeout_encoding.cc', @@ -1508,7 +1507,6 @@ 'src/core/lib/transport/http_connect_handshaker.cc', 'src/core/lib/transport/metadata_batch.cc', 'src/core/lib/transport/parsed_metadata.cc', - 'src/core/lib/transport/pid_controller.cc', 'src/core/lib/transport/status_conversion.cc', 'src/core/lib/transport/tcp_connect_handshaker.cc', 'src/core/lib/transport/timeout_encoding.cc', diff --git a/package.xml b/package.xml index ea7259e4491..67d5577eff9 100644 --- a/package.xml +++ b/package.xml @@ -1882,8 +1882,6 @@ - - diff --git a/src/core/BUILD b/src/core/BUILD index 5c2a93943df..fcb11452d18 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -2609,20 +2609,6 @@ grpc_cc_library( ], ) -grpc_cc_library( - name = "pid_controller", - srcs = [ - "lib/transport/pid_controller.cc", - ], - hdrs = [ - "lib/transport/pid_controller.h", - ], - deps = [ - "useful", - "//:gpr_platform", - ], -) - grpc_cc_library( name = "bdp_estimator", srcs = [ @@ -5907,7 +5893,6 @@ grpc_cc_library( "experiments", "http2_settings", "memory_quota", - "pid_controller", "time", "useful", "//:gpr", diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 407b15ea7de..7c3c7dcd6ec 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -646,7 +646,7 @@ static void read_channel_args(grpc_chttp2_transport* t, t->max_concurrent_streams_overload_protection = channel_args.GetBool(GRPC_ARG_MAX_CONCURRENT_STREAMS_OVERLOAD_PROTECTION) - .value_or(grpc_core::IsOverloadProtectionEnabled()); + .value_or(true); } static void init_keepalive_pings_if_enabled_locked( diff --git a/src/core/ext/transport/chttp2/transport/flow_control.cc b/src/core/ext/transport/chttp2/transport/flow_control.cc index 986c62bc427..20031119b3f 100644 --- a/src/core/ext/transport/chttp2/transport/flow_control.cc +++ b/src/core/ext/transport/chttp2/transport/flow_control.cc @@ -104,16 +104,7 @@ TransportFlowControl::TransportFlowControl(absl::string_view name, MemoryOwner* memory_owner) : memory_owner_(memory_owner), enable_bdp_probe_(enable_bdp_probe), - bdp_estimator_(name), - pid_controller_(PidController::Args() - .set_gain_p(4) - .set_gain_i(8) - .set_gain_d(0) - .set_initial_control_value(TargetLogBdp()) - .set_min_control_value(-1) - .set_max_control_value(25) - .set_integral_range(10)), - last_pid_update_(Timestamp::Now()) {} + bdp_estimator_(name) {} uint32_t TransportFlowControl::DesiredAnnounceSize(bool writing_anyway) const { const uint32_t target_announced_window = @@ -184,41 +175,6 @@ FlowControlAction TransportFlowControl::UpdateAction(FlowControlAction action) { return action; } -// Take in a target and modifies it based on the memory pressure of the system -static double AdjustForMemoryPressure(double memory_pressure, double target) { - // do not increase window under heavy memory pressure. - static const double kLowMemPressure = 0.1; - static const double kZeroTarget = 22; - static const double kHighMemPressure = 0.8; - static const double kMaxMemPressure = 0.9; - if (memory_pressure < kLowMemPressure && target < kZeroTarget) { - target = (target - kZeroTarget) * memory_pressure / kLowMemPressure + - kZeroTarget; - } else if (memory_pressure > kHighMemPressure) { - target *= 1 - std::min(1.0, (memory_pressure - kHighMemPressure) / - (kMaxMemPressure - kHighMemPressure)); - } - return target; -} - -double TransportFlowControl::TargetLogBdp() { - return AdjustForMemoryPressure( - memory_owner_->is_valid() - ? memory_owner_->GetPressureInfo().pressure_control_value - : 0.0, - 1 + log2(bdp_estimator_.EstimateBdp())); -} - -double TransportFlowControl::SmoothLogBdp(double value) { - Timestamp now = Timestamp::Now(); - double bdp_error = value - pid_controller_.last_control_value(); - const double dt = (now - last_pid_update_).seconds(); - last_pid_update_ = now; - // Limit dt to 100ms - const double kMaxDt = 0.1; - return pid_controller_.Update(bdp_error, dt > kMaxDt ? kMaxDt : dt); -} - double TransportFlowControl::TargetInitialWindowSizeBasedOnMemoryPressureAndBdp() const { @@ -323,10 +279,8 @@ FlowControlAction TransportFlowControl::PeriodicUpdate() { // TODO(ncteisen): experiment with setting target to be huge under low // memory pressure. uint32_t target = static_cast(RoundUpToPowerOf2( - Clamp(IsMemoryPressureControllerEnabled() - ? TargetInitialWindowSizeBasedOnMemoryPressureAndBdp() - : pow(2, SmoothLogBdp(TargetLogBdp())), - 0.0, static_cast(kMaxInitialWindowSize)))); + Clamp(TargetInitialWindowSizeBasedOnMemoryPressureAndBdp(), 0.0, + static_cast(kMaxInitialWindowSize)))); if (target < kMinPositiveInitialWindowSize) target = 0; if (g_test_only_transport_target_window_estimates_mocker != nullptr) { // Hook for simulating unusual flow control situations in tests. diff --git a/src/core/ext/transport/chttp2/transport/flow_control.h b/src/core/ext/transport/chttp2/transport/flow_control.h index 599da6c85e8..f155ad34e6e 100644 --- a/src/core/ext/transport/chttp2/transport/flow_control.h +++ b/src/core/ext/transport/chttp2/transport/flow_control.h @@ -40,7 +40,6 @@ #include "src/core/lib/gprpp/time.h" #include "src/core/lib/resource_quota/memory_quota.h" #include "src/core/lib/transport/bdp_estimator.h" -#include "src/core/lib/transport/pid_controller.h" extern grpc_core::TraceFlag grpc_flowctl_trace; @@ -330,8 +329,6 @@ class TransportFlowControl final { } private: - double TargetLogBdp(); - double SmoothLogBdp(double value); double TargetInitialWindowSizeBasedOnMemoryPressureAndBdp() const; static void UpdateSetting(grpc_chttp2_setting_id id, int64_t* desired_value, uint32_t new_desired_value, @@ -359,10 +356,6 @@ class TransportFlowControl final { // bdp estimation BdpEstimator bdp_estimator_; - // pid controller - PidController pid_controller_; - Timestamp last_pid_update_; - int64_t remote_window_ = kDefaultWindow; int64_t target_initial_window_size_ = kDefaultWindow; int64_t target_frame_size_ = kDefaultFrameSize; diff --git a/src/core/ext/transport/chttp2/transport/parsing.cc b/src/core/ext/transport/chttp2/transport/parsing.cc index efa216fcf6e..7ccdffee984 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.cc +++ b/src/core/ext/transport/chttp2/transport/parsing.cc @@ -674,7 +674,6 @@ static grpc_error_handle init_header_frame_parser(grpc_chttp2_transport* t, grpc_chttp2_initiate_write(t, GRPC_CHTTP2_INITIATE_WRITE_RST_STREAM); return init_header_skip_frame_parser(t, priority_type, is_eoh); } else if (GPR_UNLIKELY( - grpc_core::IsRedMaxConcurrentStreamsEnabled() && t->stream_map.size() >= t->max_concurrent_streams_policy.AdvertiseValue() && grpc_core::RandomEarlyDetection( diff --git a/src/core/lib/experiments/experiments.cc b/src/core/lib/experiments/experiments.cc index 366f712a4f8..3c40e129ae1 100644 --- a/src/core/lib/experiments/experiments.cc +++ b/src/core/lib/experiments/experiments.cc @@ -60,19 +60,12 @@ 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 = "{}"; -const char* const description_memory_pressure_controller = - "New memory pressure controller"; -const char* const additional_constraints_memory_pressure_controller = "{}"; const char* const description_monitoring_experiment = "Placeholder experiment to prove/disprove our monitoring is working"; const char* const additional_constraints_monitoring_experiment = "{}"; const char* const description_multiping = "Allow more than one ping to be in flight at a time by default."; const char* const additional_constraints_multiping = "{}"; -const char* const description_overload_protection = - "If chttp2 has more streams than it can handle open, send RST_STREAM " - "immediately on new streams appearing."; -const char* const additional_constraints_overload_protection = "{}"; 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 " @@ -97,10 +90,6 @@ const char* const description_promise_based_server_call = "If set, use the new gRPC promise based call code when it's appropriate " "(ie when all filters in a stack are promise based)"; const char* const additional_constraints_promise_based_server_call = "{}"; -const char* const description_red_max_concurrent_streams = - "Perform random early rejection of requests that would exceed a newly " - "reduced MAX_CONCURRENT_STREAMS but are allowed by the current."; -const char* const additional_constraints_red_max_concurrent_streams = "{}"; const char* const description_registered_method_lookup_in_transport = "Change registered method's lookup point to transport"; const char* const additional_constraints_registered_method_lookup_in_transport = @@ -219,15 +208,10 @@ const ExperimentMetadata g_experiment_metadata[] = { additional_constraints_keepalive_fix, nullptr, 0, false, false}, {"keepalive_server_fix", description_keepalive_server_fix, additional_constraints_keepalive_server_fix, nullptr, 0, false, false}, - {"memory_pressure_controller", description_memory_pressure_controller, - additional_constraints_memory_pressure_controller, nullptr, 0, false, - true}, {"monitoring_experiment", description_monitoring_experiment, additional_constraints_monitoring_experiment, nullptr, 0, true, true}, {"multiping", description_multiping, additional_constraints_multiping, nullptr, 0, false, true}, - {"overload_protection", description_overload_protection, - additional_constraints_overload_protection, nullptr, 0, true, true}, {"peer_state_based_framing", description_peer_state_based_framing, additional_constraints_peer_state_based_framing, nullptr, 0, false, true}, {"pending_queue_cap", description_pending_queue_cap, @@ -238,9 +222,6 @@ const ExperimentMetadata g_experiment_metadata[] = { additional_constraints_promise_based_client_call, nullptr, 0, false, true}, {"promise_based_server_call", description_promise_based_server_call, additional_constraints_promise_based_server_call, nullptr, 0, false, true}, - {"red_max_concurrent_streams", description_red_max_concurrent_streams, - additional_constraints_red_max_concurrent_streams, nullptr, 0, false, - true}, {"registered_method_lookup_in_transport", description_registered_method_lookup_in_transport, additional_constraints_registered_method_lookup_in_transport, nullptr, 0, @@ -339,19 +320,12 @@ 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 = "{}"; -const char* const description_memory_pressure_controller = - "New memory pressure controller"; -const char* const additional_constraints_memory_pressure_controller = "{}"; const char* const description_monitoring_experiment = "Placeholder experiment to prove/disprove our monitoring is working"; const char* const additional_constraints_monitoring_experiment = "{}"; const char* const description_multiping = "Allow more than one ping to be in flight at a time by default."; const char* const additional_constraints_multiping = "{}"; -const char* const description_overload_protection = - "If chttp2 has more streams than it can handle open, send RST_STREAM " - "immediately on new streams appearing."; -const char* const additional_constraints_overload_protection = "{}"; 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 " @@ -376,10 +350,6 @@ const char* const description_promise_based_server_call = "If set, use the new gRPC promise based call code when it's appropriate " "(ie when all filters in a stack are promise based)"; const char* const additional_constraints_promise_based_server_call = "{}"; -const char* const description_red_max_concurrent_streams = - "Perform random early rejection of requests that would exceed a newly " - "reduced MAX_CONCURRENT_STREAMS but are allowed by the current."; -const char* const additional_constraints_red_max_concurrent_streams = "{}"; const char* const description_registered_method_lookup_in_transport = "Change registered method's lookup point to transport"; const char* const additional_constraints_registered_method_lookup_in_transport = @@ -498,15 +468,10 @@ const ExperimentMetadata g_experiment_metadata[] = { additional_constraints_keepalive_fix, nullptr, 0, false, false}, {"keepalive_server_fix", description_keepalive_server_fix, additional_constraints_keepalive_server_fix, nullptr, 0, false, false}, - {"memory_pressure_controller", description_memory_pressure_controller, - additional_constraints_memory_pressure_controller, nullptr, 0, false, - true}, {"monitoring_experiment", description_monitoring_experiment, additional_constraints_monitoring_experiment, nullptr, 0, true, true}, {"multiping", description_multiping, additional_constraints_multiping, nullptr, 0, false, true}, - {"overload_protection", description_overload_protection, - additional_constraints_overload_protection, nullptr, 0, true, true}, {"peer_state_based_framing", description_peer_state_based_framing, additional_constraints_peer_state_based_framing, nullptr, 0, false, true}, {"pending_queue_cap", description_pending_queue_cap, @@ -517,9 +482,6 @@ const ExperimentMetadata g_experiment_metadata[] = { additional_constraints_promise_based_client_call, nullptr, 0, false, true}, {"promise_based_server_call", description_promise_based_server_call, additional_constraints_promise_based_server_call, nullptr, 0, false, true}, - {"red_max_concurrent_streams", description_red_max_concurrent_streams, - additional_constraints_red_max_concurrent_streams, nullptr, 0, false, - true}, {"registered_method_lookup_in_transport", description_registered_method_lookup_in_transport, additional_constraints_registered_method_lookup_in_transport, nullptr, 0, @@ -618,19 +580,12 @@ 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 = "{}"; -const char* const description_memory_pressure_controller = - "New memory pressure controller"; -const char* const additional_constraints_memory_pressure_controller = "{}"; const char* const description_monitoring_experiment = "Placeholder experiment to prove/disprove our monitoring is working"; const char* const additional_constraints_monitoring_experiment = "{}"; const char* const description_multiping = "Allow more than one ping to be in flight at a time by default."; const char* const additional_constraints_multiping = "{}"; -const char* const description_overload_protection = - "If chttp2 has more streams than it can handle open, send RST_STREAM " - "immediately on new streams appearing."; -const char* const additional_constraints_overload_protection = "{}"; 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 " @@ -655,10 +610,6 @@ const char* const description_promise_based_server_call = "If set, use the new gRPC promise based call code when it's appropriate " "(ie when all filters in a stack are promise based)"; const char* const additional_constraints_promise_based_server_call = "{}"; -const char* const description_red_max_concurrent_streams = - "Perform random early rejection of requests that would exceed a newly " - "reduced MAX_CONCURRENT_STREAMS but are allowed by the current."; -const char* const additional_constraints_red_max_concurrent_streams = "{}"; const char* const description_registered_method_lookup_in_transport = "Change registered method's lookup point to transport"; const char* const additional_constraints_registered_method_lookup_in_transport = @@ -777,15 +728,10 @@ const ExperimentMetadata g_experiment_metadata[] = { additional_constraints_keepalive_fix, nullptr, 0, false, false}, {"keepalive_server_fix", description_keepalive_server_fix, additional_constraints_keepalive_server_fix, nullptr, 0, false, false}, - {"memory_pressure_controller", description_memory_pressure_controller, - additional_constraints_memory_pressure_controller, nullptr, 0, false, - true}, {"monitoring_experiment", description_monitoring_experiment, additional_constraints_monitoring_experiment, nullptr, 0, true, true}, {"multiping", description_multiping, additional_constraints_multiping, nullptr, 0, false, true}, - {"overload_protection", description_overload_protection, - additional_constraints_overload_protection, nullptr, 0, true, true}, {"peer_state_based_framing", description_peer_state_based_framing, additional_constraints_peer_state_based_framing, nullptr, 0, false, true}, {"pending_queue_cap", description_pending_queue_cap, @@ -796,9 +742,6 @@ const ExperimentMetadata g_experiment_metadata[] = { additional_constraints_promise_based_client_call, nullptr, 0, false, true}, {"promise_based_server_call", description_promise_based_server_call, additional_constraints_promise_based_server_call, nullptr, 0, false, true}, - {"red_max_concurrent_streams", description_red_max_concurrent_streams, - additional_constraints_red_max_concurrent_streams, nullptr, 0, false, - true}, {"registered_method_lookup_in_transport", description_registered_method_lookup_in_transport, additional_constraints_registered_method_lookup_in_transport, nullptr, 0, diff --git a/src/core/lib/experiments/experiments.h b/src/core/lib/experiments/experiments.h index e4397dc9576..d631bf357a4 100644 --- a/src/core/lib/experiments/experiments.h +++ b/src/core/lib/experiments/experiments.h @@ -79,12 +79,9 @@ inline bool IsFreeLargeAllocatorEnabled() { return false; } inline bool IsHttp2StatsFixEnabled() { return true; } inline bool IsKeepaliveFixEnabled() { return false; } inline bool IsKeepaliveServerFixEnabled() { return false; } -inline bool IsMemoryPressureControllerEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_MONITORING_EXPERIMENT inline bool IsMonitoringExperimentEnabled() { return true; } inline bool IsMultipingEnabled() { return false; } -#define GRPC_EXPERIMENT_IS_INCLUDED_OVERLOAD_PROTECTION -inline bool IsOverloadProtectionEnabled() { return true; } inline bool IsPeerStateBasedFramingEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_PENDING_QUEUE_CAP inline bool IsPendingQueueCapEnabled() { return true; } @@ -92,7 +89,6 @@ inline bool IsPendingQueueCapEnabled() { return true; } inline bool IsPickFirstHappyEyeballsEnabled() { return true; } inline bool IsPromiseBasedClientCallEnabled() { return false; } inline bool IsPromiseBasedServerCallEnabled() { return false; } -inline bool IsRedMaxConcurrentStreamsEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_REGISTERED_METHOD_LOOKUP_IN_TRANSPORT inline bool IsRegisteredMethodLookupInTransportEnabled() { return true; } inline bool IsPromiseBasedInprocTransportEnabled() { return false; } @@ -145,12 +141,9 @@ inline bool IsFreeLargeAllocatorEnabled() { return false; } inline bool IsHttp2StatsFixEnabled() { return true; } inline bool IsKeepaliveFixEnabled() { return false; } inline bool IsKeepaliveServerFixEnabled() { return false; } -inline bool IsMemoryPressureControllerEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_MONITORING_EXPERIMENT inline bool IsMonitoringExperimentEnabled() { return true; } inline bool IsMultipingEnabled() { return false; } -#define GRPC_EXPERIMENT_IS_INCLUDED_OVERLOAD_PROTECTION -inline bool IsOverloadProtectionEnabled() { return true; } inline bool IsPeerStateBasedFramingEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_PENDING_QUEUE_CAP inline bool IsPendingQueueCapEnabled() { return true; } @@ -158,7 +151,6 @@ inline bool IsPendingQueueCapEnabled() { return true; } inline bool IsPickFirstHappyEyeballsEnabled() { return true; } inline bool IsPromiseBasedClientCallEnabled() { return false; } inline bool IsPromiseBasedServerCallEnabled() { return false; } -inline bool IsRedMaxConcurrentStreamsEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_REGISTERED_METHOD_LOOKUP_IN_TRANSPORT inline bool IsRegisteredMethodLookupInTransportEnabled() { return true; } inline bool IsPromiseBasedInprocTransportEnabled() { return false; } @@ -211,12 +203,9 @@ inline bool IsFreeLargeAllocatorEnabled() { return false; } inline bool IsHttp2StatsFixEnabled() { return true; } inline bool IsKeepaliveFixEnabled() { return false; } inline bool IsKeepaliveServerFixEnabled() { return false; } -inline bool IsMemoryPressureControllerEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_MONITORING_EXPERIMENT inline bool IsMonitoringExperimentEnabled() { return true; } inline bool IsMultipingEnabled() { return false; } -#define GRPC_EXPERIMENT_IS_INCLUDED_OVERLOAD_PROTECTION -inline bool IsOverloadProtectionEnabled() { return true; } inline bool IsPeerStateBasedFramingEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_PENDING_QUEUE_CAP inline bool IsPendingQueueCapEnabled() { return true; } @@ -224,7 +213,6 @@ inline bool IsPendingQueueCapEnabled() { return true; } inline bool IsPickFirstHappyEyeballsEnabled() { return true; } inline bool IsPromiseBasedClientCallEnabled() { return false; } inline bool IsPromiseBasedServerCallEnabled() { return false; } -inline bool IsRedMaxConcurrentStreamsEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_REGISTERED_METHOD_LOOKUP_IN_TRANSPORT inline bool IsRegisteredMethodLookupInTransportEnabled() { return true; } inline bool IsPromiseBasedInprocTransportEnabled() { return false; } @@ -267,16 +255,13 @@ enum ExperimentIds { kExperimentIdHttp2StatsFix, kExperimentIdKeepaliveFix, kExperimentIdKeepaliveServerFix, - kExperimentIdMemoryPressureController, kExperimentIdMonitoringExperiment, kExperimentIdMultiping, - kExperimentIdOverloadProtection, kExperimentIdPeerStateBasedFraming, kExperimentIdPendingQueueCap, kExperimentIdPickFirstHappyEyeballs, kExperimentIdPromiseBasedClientCall, kExperimentIdPromiseBasedServerCall, - kExperimentIdRedMaxConcurrentStreams, kExperimentIdRegisteredMethodLookupInTransport, kExperimentIdPromiseBasedInprocTransport, kExperimentIdRegisteredMethodsMap, @@ -344,10 +329,6 @@ inline bool IsKeepaliveFixEnabled() { inline bool IsKeepaliveServerFixEnabled() { return IsExperimentEnabled(kExperimentIdKeepaliveServerFix); } -#define GRPC_EXPERIMENT_IS_INCLUDED_MEMORY_PRESSURE_CONTROLLER -inline bool IsMemoryPressureControllerEnabled() { - return IsExperimentEnabled(kExperimentIdMemoryPressureController); -} #define GRPC_EXPERIMENT_IS_INCLUDED_MONITORING_EXPERIMENT inline bool IsMonitoringExperimentEnabled() { return IsExperimentEnabled(kExperimentIdMonitoringExperiment); @@ -356,10 +337,6 @@ inline bool IsMonitoringExperimentEnabled() { inline bool IsMultipingEnabled() { return IsExperimentEnabled(kExperimentIdMultiping); } -#define GRPC_EXPERIMENT_IS_INCLUDED_OVERLOAD_PROTECTION -inline bool IsOverloadProtectionEnabled() { - return IsExperimentEnabled(kExperimentIdOverloadProtection); -} #define GRPC_EXPERIMENT_IS_INCLUDED_PEER_STATE_BASED_FRAMING inline bool IsPeerStateBasedFramingEnabled() { return IsExperimentEnabled(kExperimentIdPeerStateBasedFraming); @@ -380,10 +357,6 @@ inline bool IsPromiseBasedClientCallEnabled() { inline bool IsPromiseBasedServerCallEnabled() { return IsExperimentEnabled(kExperimentIdPromiseBasedServerCall); } -#define GRPC_EXPERIMENT_IS_INCLUDED_RED_MAX_CONCURRENT_STREAMS -inline bool IsRedMaxConcurrentStreamsEnabled() { - return IsExperimentEnabled(kExperimentIdRedMaxConcurrentStreams); -} #define GRPC_EXPERIMENT_IS_INCLUDED_REGISTERED_METHOD_LOOKUP_IN_TRANSPORT inline bool IsRegisteredMethodLookupInTransportEnabled() { return IsExperimentEnabled(kExperimentIdRegisteredMethodLookupInTransport); diff --git a/src/core/lib/experiments/experiments.yaml b/src/core/lib/experiments/experiments.yaml index 23ec1fb0325..7c92753a819 100644 --- a/src/core/lib/experiments/experiments.yaml +++ b/src/core/lib/experiments/experiments.yaml @@ -107,11 +107,6 @@ owner: yashkt@google.com test_tags: [] allow_in_fuzzing_config: false -- name: memory_pressure_controller - description: New memory pressure controller - expiry: 2024/05/05 - owner: ctiller@google.com - test_tags: [resource_quota_test] - name: monitoring_experiment description: Placeholder experiment to prove/disprove our monitoring is working expiry: never-ever @@ -123,13 +118,6 @@ expiry: 2024/01/15 owner: ctiller@google.com test_tags: [flow_control_test] -- name: overload_protection - description: - If chttp2 has more streams than it can handle open, send RST_STREAM immediately - on new streams appearing. - expiry: 2024/03/03 - owner: ctiller@google.com - test_tags: [flow_control_test] - name: peer_state_based_framing description: If set, the max sizes of frames sent to lower layers is controlled based @@ -177,13 +165,6 @@ expiry: 2024/06/14 owner: ctiller@google.com test_tags: ["core_end2end_test", "cpp_end2end_test", "xds_end2end_test", "logging_test"] -- name: red_max_concurrent_streams - description: - Perform random early rejection of requests that would exceed a newly reduced - MAX_CONCURRENT_STREAMS but are allowed by the current. - expiry: 2024/03/03 - owner: ctiller@google.com - test_tags: [flow_control_test] - name: registered_method_lookup_in_transport description: Change registered method's lookup point to transport diff --git a/src/core/lib/experiments/rollouts.yaml b/src/core/lib/experiments/rollouts.yaml index 160a12e55a0..a49bcd7e6af 100644 --- a/src/core/lib/experiments/rollouts.yaml +++ b/src/core/lib/experiments/rollouts.yaml @@ -78,12 +78,8 @@ default: false - name: keepalive_server_fix default: false -- name: memory_pressure_controller - default: false - name: monitoring_experiment default: true -- name: overload_protection - default: true - name: peer_state_based_framing default: false - name: pending_queue_cap @@ -94,8 +90,6 @@ default: false - name: promise_based_server_call default: false -- name: red_max_concurrent_streams - default: false - name: registered_method_lookup_in_transport default: true - name: registered_methods_map diff --git a/src/core/lib/resource_quota/memory_quota.cc b/src/core/lib/resource_quota/memory_quota.cc index 0e63e45b4df..fb3542b9dc4 100644 --- a/src/core/lib/resource_quota/memory_quota.cc +++ b/src/core/lib/resource_quota/memory_quota.cc @@ -658,14 +658,9 @@ BasicMemoryQuota::PressureInfo BasicMemoryQuota::GetPressureInfo() { if (size < 1) return PressureInfo{1, 1, 1}; PressureInfo pressure_info; pressure_info.instantaneous_pressure = std::max(0.0, (size - free) / size); - if (IsMemoryPressureControllerEnabled()) { - pressure_info.pressure_control_value = - pressure_tracker_.AddSampleAndGetControlValue( - pressure_info.instantaneous_pressure); - } else { - pressure_info.pressure_control_value = - std::min(pressure_info.instantaneous_pressure, 1.0); - } + pressure_info.pressure_control_value = + pressure_tracker_.AddSampleAndGetControlValue( + pressure_info.instantaneous_pressure); pressure_info.max_recommended_allocation_size = quota_size / 16; return pressure_info; } diff --git a/src/core/lib/transport/pid_controller.cc b/src/core/lib/transport/pid_controller.cc deleted file mode 100644 index ca654b7d05c..00000000000 --- a/src/core/lib/transport/pid_controller.cc +++ /dev/null @@ -1,51 +0,0 @@ -// -// -// Copyright 2016 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 - -#include "src/core/lib/transport/pid_controller.h" - -#include "src/core/lib/gpr/useful.h" - -namespace grpc_core { - -PidController::PidController(const Args& args) - : last_control_value_(args.initial_control_value()), args_(args) {} - -double PidController::Update(double error, double dt) { - if (dt <= 0) return last_control_value_; - // integrate error using the trapezoid rule - error_integral_ += dt * (last_error_ + error) * 0.5; - error_integral_ = - Clamp(error_integral_, -args_.integral_range(), args_.integral_range()); - double diff_error = (error - last_error_) / dt; - // calculate derivative of control value vs time - double dc_dt = args_.gain_p() * error + args_.gain_i() * error_integral_ + - args_.gain_d() * diff_error; - // and perform trapezoidal integration - double new_control_value = - last_control_value_ + dt * (last_dc_dt_ + dc_dt) * 0.5; - new_control_value = Clamp(new_control_value, args_.min_control_value(), - args_.max_control_value()); - last_error_ = error; - last_dc_dt_ = dc_dt; - last_control_value_ = new_control_value; - return new_control_value; -} - -} // namespace grpc_core diff --git a/src/core/lib/transport/pid_controller.h b/src/core/lib/transport/pid_controller.h deleted file mode 100644 index 856c78c667e..00000000000 --- a/src/core/lib/transport/pid_controller.h +++ /dev/null @@ -1,116 +0,0 @@ -// -// -// Copyright 2016 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. -// -// - -#ifndef GRPC_SRC_CORE_LIB_TRANSPORT_PID_CONTROLLER_H -#define GRPC_SRC_CORE_LIB_TRANSPORT_PID_CONTROLLER_H - -#include - -#include - -// \file Simple PID controller. -// Implements a proportional-integral-derivative controller. -// Used when we want to iteratively control a variable to converge some other -// observed value to a 'set-point'. -// Gains can be set to adjust sensitivity to current error (p), the integral -// of error (i), and the derivative of error (d). - -namespace grpc_core { - -class PidController { - public: - class Args { - public: - double gain_p() const { return gain_p_; } - double gain_i() const { return gain_i_; } - double gain_d() const { return gain_d_; } - double initial_control_value() const { return initial_control_value_; } - double min_control_value() const { return min_control_value_; } - double max_control_value() const { return max_control_value_; } - double integral_range() const { return integral_range_; } - - Args& set_gain_p(double gain_p) { - gain_p_ = gain_p; - return *this; - } - Args& set_gain_i(double gain_i) { - gain_i_ = gain_i; - return *this; - } - Args& set_gain_d(double gain_d) { - gain_d_ = gain_d; - return *this; - } - Args& set_initial_control_value(double initial_control_value) { - initial_control_value_ = initial_control_value; - return *this; - } - Args& set_min_control_value(double min_control_value) { - min_control_value_ = min_control_value; - return *this; - } - Args& set_max_control_value(double max_control_value) { - max_control_value_ = max_control_value; - return *this; - } - Args& set_integral_range(double integral_range) { - integral_range_ = integral_range; - return *this; - } - - private: - double gain_p_ = 0.0; - double gain_i_ = 0.0; - double gain_d_ = 0.0; - double initial_control_value_ = 0.0; - double min_control_value_ = std::numeric_limits::min(); - double max_control_value_ = std::numeric_limits::max(); - double integral_range_ = std::numeric_limits::max(); - }; - - explicit PidController(const Args& args); - - /// Reset the controller internal state: useful when the environment has - /// changed significantly - void Reset() { - last_error_ = 0.0; - last_dc_dt_ = 0.0; - error_integral_ = 0.0; - } - - /// Update the controller: given a current error estimate, and the time since - /// the last update, returns a new control value - double Update(double error, double dt); - - /// Returns the last control value calculated - double last_control_value() const { return last_control_value_; } - - /// Returns the current error integral (mostly for testing) - double error_integral() const { return error_integral_; } - - private: - double last_error_ = 0.0; - double error_integral_ = 0.0; - double last_control_value_; - double last_dc_dt_ = 0.0; - const Args args_; -}; - -} // namespace grpc_core - -#endif // GRPC_SRC_CORE_LIB_TRANSPORT_PID_CONTROLLER_H diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 971a75742d4..351065b21a8 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -811,7 +811,6 @@ CORE_SOURCE_FILES = [ 'src/core/lib/transport/http_connect_handshaker.cc', 'src/core/lib/transport/metadata_batch.cc', 'src/core/lib/transport/parsed_metadata.cc', - 'src/core/lib/transport/pid_controller.cc', 'src/core/lib/transport/status_conversion.cc', 'src/core/lib/transport/tcp_connect_handshaker.cc', 'src/core/lib/transport/timeout_encoding.cc', diff --git a/test/core/transport/BUILD b/test/core/transport/BUILD index d42792f9093..df6c57785c4 100644 --- a/test/core/transport/BUILD +++ b/test/core/transport/BUILD @@ -94,21 +94,6 @@ grpc_cc_test( ], ) -grpc_cc_test( - name = "pid_controller_test", - srcs = ["pid_controller_test.cc"], - external_deps = [ - "gtest", - ], - language = "C++", - deps = [ - "//:gpr", - "//:grpc", - "//test/core/util:grpc_test_util", - "//test/core/util:grpc_test_util_base", - ], -) - grpc_cc_test( name = "promise_endpoint_test", srcs = ["promise_endpoint_test.cc"], diff --git a/test/core/transport/pid_controller_test.cc b/test/core/transport/pid_controller_test.cc deleted file mode 100644 index 1d7cedcb8db..00000000000 --- a/test/core/transport/pid_controller_test.cc +++ /dev/null @@ -1,89 +0,0 @@ -// -// -// Copyright 2016 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 "src/core/lib/transport/pid_controller.h" - -#include - -#include -#include - -#include "gtest/gtest.h" - -#include "test/core/util/test_config.h" - -namespace grpc_core { -namespace testing { - -TEST(PidController, NoOp) { - PidController pid(PidController::Args() - .set_gain_p(1) - .set_gain_i(1) - .set_gain_d(1) - .set_initial_control_value(1)); -} - -struct SimpleConvergenceTestArgs { - double gain_p; - double gain_i; - double gain_d; - double dt; - double set_point; - double start; -}; - -std::ostream& operator<<(std::ostream& out, SimpleConvergenceTestArgs args) { - return out << "gain_p:" << args.gain_p << " gain_i:" << args.gain_i - << " gain_d:" << args.gain_d << " dt:" << args.dt - << " set_point:" << args.set_point << " start:" << args.start; -} - -class SimpleConvergenceTest - : public ::testing::TestWithParam {}; - -TEST_P(SimpleConvergenceTest, Converges) { - PidController pid(PidController::Args() - .set_gain_p(GetParam().gain_p) - .set_gain_i(GetParam().gain_i) - .set_gain_d(GetParam().gain_d) - .set_initial_control_value(GetParam().start)); - - for (int i = 0; i < 100000; i++) { - pid.Update(GetParam().set_point - pid.last_control_value(), GetParam().dt); - } - - EXPECT_LT(fabs(GetParam().set_point - pid.last_control_value()), 0.1); - if (GetParam().gain_i > 0) { - EXPECT_LT(fabs(pid.error_integral()), 0.1); - } -} - -INSTANTIATE_TEST_SUITE_P( - X, SimpleConvergenceTest, - ::testing::Values(SimpleConvergenceTestArgs{0.2, 0, 0, 1, 100, 0}, - SimpleConvergenceTestArgs{0.2, 0.1, 0, 1, 100, 0}, - SimpleConvergenceTestArgs{0.2, 0.1, 0.1, 1, 100, 0})); - -} // namespace testing -} // namespace grpc_core - -int main(int argc, char** argv) { - grpc::testing::TestEnvironment env(&argc, argv); - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index dc859f23350..c0fc6f9e749 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2899,8 +2899,6 @@ src/core/lib/transport/metadata_batch.h \ src/core/lib/transport/metadata_compression_traits.h \ src/core/lib/transport/parsed_metadata.cc \ src/core/lib/transport/parsed_metadata.h \ -src/core/lib/transport/pid_controller.cc \ -src/core/lib/transport/pid_controller.h \ src/core/lib/transport/simple_slice_based_metadata.h \ src/core/lib/transport/status_conversion.cc \ src/core/lib/transport/status_conversion.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index e3924472666..1645a7c7c9a 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -2680,8 +2680,6 @@ src/core/lib/transport/metadata_batch.h \ src/core/lib/transport/metadata_compression_traits.h \ src/core/lib/transport/parsed_metadata.cc \ src/core/lib/transport/parsed_metadata.h \ -src/core/lib/transport/pid_controller.cc \ -src/core/lib/transport/pid_controller.h \ src/core/lib/transport/simple_slice_based_metadata.h \ src/core/lib/transport/status_conversion.cc \ src/core/lib/transport/status_conversion.h \ diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 665d030a6fb..7981a98a90b 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -6601,30 +6601,6 @@ ], "uses_polling": false }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": true, - "language": "c++", - "name": "pid_controller_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": true - }, { "args": [], "benchmark": false,