From 5fcc5f8a436c7c967f52f533932f0d05ea0aded3 Mon Sep 17 00:00:00 2001 From: Vignesh Babu Date: Fri, 8 Nov 2024 13:43:18 -0800 Subject: [PATCH] [Chttp2Transport] Flush data out over the transport quickly under high memory pressure. The change is protected under an experiment: disable_buffer_hint_on_high_memory_pressure PiperOrigin-RevId: 694625426 --- bazel/experiments.bzl | 1 + .../chttp2/transport/chttp2_transport.cc | 16 ++++++++++- src/core/lib/experiments/experiments.cc | 27 +++++++++++++++++++ src/core/lib/experiments/experiments.h | 9 +++++++ src/core/lib/experiments/experiments.yaml | 6 +++++ src/core/lib/experiments/rollouts.yaml | 2 ++ 6 files changed, 60 insertions(+), 1 deletion(-) diff --git a/bazel/experiments.bzl b/bazel/experiments.bzl index b2ccd14adf280..14c2f66a5df25 100644 --- a/bazel/experiments.bzl +++ b/bazel/experiments.bzl @@ -21,6 +21,7 @@ EXPERIMENT_ENABLES = { "canary_client_privacy": "canary_client_privacy", "chaotic_good_legacy_protocol": "chaotic_good_legacy_protocol", "client_privacy": "client_privacy", + "disable_buffer_hint_on_high_memory_pressure": "disable_buffer_hint_on_high_memory_pressure", "event_engine_application_callbacks": "event_engine_application_callbacks", "event_engine_callback_cq": "event_engine_application_callbacks,event_engine_callback_cq", "event_engine_client": "event_engine_client", diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index d4817366fb9f8..3dfd962a85bc8 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -1470,7 +1471,7 @@ static void send_message_locked( op->payload->send_message.send_message->Length()); on_complete->next_data.scratch |= t->closure_barrier_may_cover_write; s->send_message_finished = add_closure_barrier(op->on_complete); - const uint32_t flags = op_payload->send_message.flags; + uint32_t flags = 0; if (s->write_closed) { op->payload->send_message.stream_write_closed = true; // We should NOT return an error here, so as to avoid a cancel OP being @@ -1480,6 +1481,19 @@ static void send_message_locked( absl::OkStatus(), "fetching_send_message_finished"); } else { + // Buffer hint is used to buffer the message in the transport until the + // write buffer size (specified through GRPC_ARG_HTTP2_WRITE_BUFFER_SIZE) is + // reached. This is to batch writes sent down to tcp. However, if the memory + // pressure is high, disable the buffer hint to flush data down to tcp as + // soon as possible to avoid OOM. + if (grpc_core::IsDisableBufferHintOnHighMemoryPressureEnabled() && + t->memory_owner.GetPressureInfo().pressure_control_value >= 0.8) { + // Disable write buffer hint if memory pressure is high. The value of 0.8 + // is chosen to match the threshold used by the tcp endpoint (in + // allocating memory for socket reads). + op_payload->send_message.flags &= ~GRPC_WRITE_BUFFER_HINT; + } + flags = op_payload->send_message.flags; uint8_t* frame_hdr = grpc_slice_buffer_tiny_add(&s->flow_controlled_buffer, GRPC_HEADER_SIZE_IN_BYTES); frame_hdr[0] = (flags & GRPC_WRITE_INTERNAL_COMPRESS) != 0; diff --git a/src/core/lib/experiments/experiments.cc b/src/core/lib/experiments/experiments.cc index c4f1b8a464b7b..a6ac29e2feea3 100644 --- a/src/core/lib/experiments/experiments.cc +++ b/src/core/lib/experiments/experiments.cc @@ -35,6 +35,11 @@ const char* const description_chaotic_good_legacy_protocol = const char* const additional_constraints_chaotic_good_legacy_protocol = "{}"; const char* const description_client_privacy = "If set, client privacy"; const char* const additional_constraints_client_privacy = "{}"; +const char* const description_disable_buffer_hint_on_high_memory_pressure = + "Disable buffer hint flag parsing in the transport under high memory " + "pressure."; +const char* const + additional_constraints_disable_buffer_hint_on_high_memory_pressure = "{}"; const char* const description_event_engine_application_callbacks = "Run application callbacks in EventEngine threads, instead of on the " "thread-local ApplicationCallbackExecCtx"; @@ -133,6 +138,10 @@ const ExperimentMetadata g_experiment_metadata[] = { true}, {"client_privacy", description_client_privacy, additional_constraints_client_privacy, nullptr, 0, false, false}, + {"disable_buffer_hint_on_high_memory_pressure", + description_disable_buffer_hint_on_high_memory_pressure, + additional_constraints_disable_buffer_hint_on_high_memory_pressure, + nullptr, 0, false, true}, {"event_engine_application_callbacks", description_event_engine_application_callbacks, additional_constraints_event_engine_application_callbacks, nullptr, 0, @@ -207,6 +216,11 @@ const char* const description_chaotic_good_legacy_protocol = const char* const additional_constraints_chaotic_good_legacy_protocol = "{}"; const char* const description_client_privacy = "If set, client privacy"; const char* const additional_constraints_client_privacy = "{}"; +const char* const description_disable_buffer_hint_on_high_memory_pressure = + "Disable buffer hint flag parsing in the transport under high memory " + "pressure."; +const char* const + additional_constraints_disable_buffer_hint_on_high_memory_pressure = "{}"; const char* const description_event_engine_application_callbacks = "Run application callbacks in EventEngine threads, instead of on the " "thread-local ApplicationCallbackExecCtx"; @@ -305,6 +319,10 @@ const ExperimentMetadata g_experiment_metadata[] = { true}, {"client_privacy", description_client_privacy, additional_constraints_client_privacy, nullptr, 0, false, false}, + {"disable_buffer_hint_on_high_memory_pressure", + description_disable_buffer_hint_on_high_memory_pressure, + additional_constraints_disable_buffer_hint_on_high_memory_pressure, + nullptr, 0, false, true}, {"event_engine_application_callbacks", description_event_engine_application_callbacks, additional_constraints_event_engine_application_callbacks, nullptr, 0, @@ -379,6 +397,11 @@ const char* const description_chaotic_good_legacy_protocol = const char* const additional_constraints_chaotic_good_legacy_protocol = "{}"; const char* const description_client_privacy = "If set, client privacy"; const char* const additional_constraints_client_privacy = "{}"; +const char* const description_disable_buffer_hint_on_high_memory_pressure = + "Disable buffer hint flag parsing in the transport under high memory " + "pressure."; +const char* const + additional_constraints_disable_buffer_hint_on_high_memory_pressure = "{}"; const char* const description_event_engine_application_callbacks = "Run application callbacks in EventEngine threads, instead of on the " "thread-local ApplicationCallbackExecCtx"; @@ -477,6 +500,10 @@ const ExperimentMetadata g_experiment_metadata[] = { true}, {"client_privacy", description_client_privacy, additional_constraints_client_privacy, nullptr, 0, false, false}, + {"disable_buffer_hint_on_high_memory_pressure", + description_disable_buffer_hint_on_high_memory_pressure, + additional_constraints_disable_buffer_hint_on_high_memory_pressure, + nullptr, 0, false, true}, {"event_engine_application_callbacks", description_event_engine_application_callbacks, additional_constraints_event_engine_application_callbacks, nullptr, 0, diff --git a/src/core/lib/experiments/experiments.h b/src/core/lib/experiments/experiments.h index e52be7eb4d688..84089d39a8c5a 100644 --- a/src/core/lib/experiments/experiments.h +++ b/src/core/lib/experiments/experiments.h @@ -62,6 +62,7 @@ inline bool IsCallTracerInTransportEnabled() { return true; } inline bool IsCanaryClientPrivacyEnabled() { return false; } inline bool IsChaoticGoodLegacyProtocolEnabled() { return false; } inline bool IsClientPrivacyEnabled() { return false; } +inline bool IsDisableBufferHintOnHighMemoryPressureEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_APPLICATION_CALLBACKS inline bool IsEventEngineApplicationCallbacksEnabled() { return true; } #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_CALLBACK_CQ @@ -97,6 +98,7 @@ inline bool IsCallTracerInTransportEnabled() { return true; } inline bool IsCanaryClientPrivacyEnabled() { return false; } inline bool IsChaoticGoodLegacyProtocolEnabled() { return false; } inline bool IsClientPrivacyEnabled() { return false; } +inline bool IsDisableBufferHintOnHighMemoryPressureEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_APPLICATION_CALLBACKS inline bool IsEventEngineApplicationCallbacksEnabled() { return true; } #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_CALLBACK_CQ @@ -135,6 +137,7 @@ inline bool IsCallTracerInTransportEnabled() { return true; } inline bool IsCanaryClientPrivacyEnabled() { return false; } inline bool IsChaoticGoodLegacyProtocolEnabled() { return false; } inline bool IsClientPrivacyEnabled() { return false; } +inline bool IsDisableBufferHintOnHighMemoryPressureEnabled() { return false; } #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_APPLICATION_CALLBACKS inline bool IsEventEngineApplicationCallbacksEnabled() { return true; } #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_CALLBACK_CQ @@ -175,6 +178,7 @@ enum ExperimentIds { kExperimentIdCanaryClientPrivacy, kExperimentIdChaoticGoodLegacyProtocol, kExperimentIdClientPrivacy, + kExperimentIdDisableBufferHintOnHighMemoryPressure, kExperimentIdEventEngineApplicationCallbacks, kExperimentIdEventEngineCallbackCq, kExperimentIdEventEngineClient, @@ -215,6 +219,11 @@ inline bool IsChaoticGoodLegacyProtocolEnabled() { inline bool IsClientPrivacyEnabled() { return IsExperimentEnabled(); } +#define GRPC_EXPERIMENT_IS_INCLUDED_DISABLE_BUFFER_HINT_ON_HIGH_MEMORY_PRESSURE +inline bool IsDisableBufferHintOnHighMemoryPressureEnabled() { + return IsExperimentEnabled< + kExperimentIdDisableBufferHintOnHighMemoryPressure>(); +} #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_APPLICATION_CALLBACKS inline bool IsEventEngineApplicationCallbacksEnabled() { return IsExperimentEnabled(); diff --git a/src/core/lib/experiments/experiments.yaml b/src/core/lib/experiments/experiments.yaml index 11f73ebf2fb16..f81ed50cf7818 100644 --- a/src/core/lib/experiments/experiments.yaml +++ b/src/core/lib/experiments/experiments.yaml @@ -64,6 +64,12 @@ owner: alishananda@google.com test_tags: [] allow_in_fuzzing_config: false +- name: disable_buffer_hint_on_high_memory_pressure + description: + Disable buffer hint flag parsing in the transport under high memory pressure. + expiry: 2025/03/01 + owner: vigneshbabu@google.com + test_tags: [] - name: event_engine_application_callbacks description: Run application callbacks in EventEngine threads, instead of on the thread-local ApplicationCallbackExecCtx expiry: 2025/03/01 diff --git a/src/core/lib/experiments/rollouts.yaml b/src/core/lib/experiments/rollouts.yaml index e79e1806fa125..e86499aa7a57c 100644 --- a/src/core/lib/experiments/rollouts.yaml +++ b/src/core/lib/experiments/rollouts.yaml @@ -53,6 +53,8 @@ windows: broken - name: client_privacy default: false +- name: disable_buffer_hint_on_high_memory_pressure + default: false - name: event_engine_application_callbacks default: true - name: event_engine_callback_cq