You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by rr...@apache.org on 2022/05/04 15:54:33 UTC

[trafficserver] branch master updated: Add http2.default_buffer_water_mark config to tune latency (#8747)

This is an automated email from the ASF dual-hosted git repository.

rrm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 0f2d941ff Add http2.default_buffer_water_mark config to tune latency (#8747)
0f2d941ff is described below

commit 0f2d941ffd6f92e2af4127af4e4bab7cce8490e5
Author: Randall Meyer <rr...@apache.org>
AuthorDate: Wed May 4 08:54:27 2022 -0700

    Add http2.default_buffer_water_mark config to tune latency (#8747)
    
    Co-authored-by: Masaori Koshiba <ma...@apache.org>
---
 doc/admin-guide/files/records.config.en.rst |  7 +++++++
 iocore/net/UnixNetVConnection.cc            |  4 ++--
 mgmt/RecordsConfig.cc                       |  2 ++
 proxy/http2/HTTP2.cc                        |  2 ++
 proxy/http2/HTTP2.h                         |  1 +
 proxy/http2/Http2ClientSession.cc           | 10 ++++++----
 proxy/http2/Http2CommonSession.cc           |  6 ++++++
 proxy/http2/Http2CommonSession.h            |  1 +
 proxy/http2/Http2ConnectionState.cc         | 12 ++++++------
 9 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index 9da78bde0..e359a3cef 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -4286,6 +4286,13 @@ HTTP/2 Configuration
    frames. Write operation will be triggered at least once every this configured
    number of millisecond regardless of pending data size.
 
+.. ts:cv:: CONFIG proxy.config.http2.default_buffer_water_mark INT -1
+   :reloadable:
+   :units: bytes
+
+   Specifies the high water mark for all HTTP/2 frames on an outoging connection.
+   Default is -1 to preserve existing water marking behavior.
+
 HTTP/3 Configuration
 ====================
 
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index 4602e3c25..3edecc955 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -441,7 +441,7 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection *vc, EThread *thread)
   int signalled = 0;
 
   // signal write ready to allow user to fill the buffer
-  if (towrite != ntodo && buf.writer()->write_avail()) {
+  if (towrite != ntodo && !buf.writer()->high_water()) {
     if (write_signal_and_update(VC_EVENT_WRITE_READY, vc) != EVENT_CONT) {
       return;
     }
@@ -519,7 +519,7 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection *vc, EThread *thread)
     }
 
     int e = 0;
-    if (!signalled) {
+    if (!signalled || (s->vio.ntodo() > 0 && !buf.writer()->high_water())) {
       e = VC_EVENT_WRITE_READY;
     } else if (wbe_event != vc->write_buffer_empty_event) {
       // @a signalled means we won't send an event, and the event values differing means we
diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
index ee91dc9f2..9cf5bc01b 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -1394,6 +1394,8 @@ static const RecordElement RecordsConfig[] =
   ,
   {RECT_CONFIG, "proxy.config.http2.write_time_threshold", RECD_INT, "100", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
   ,
+  {RECT_CONFIG, "proxy.config.http2.default_buffer_water_mark", RECD_INT, "-1", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
+  ,
 
   //############
   //#
diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index 6959e7d22..f1360ddc6 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -812,6 +812,7 @@ uint32_t Http2::header_table_size_limit         = 65536;
 uint32_t Http2::write_buffer_block_size         = 262144;
 float Http2::write_size_threshold               = 0.5;
 uint32_t Http2::write_time_threshold            = 100;
+uint32_t Http2::buffer_water_mark               = 0;
 
 void
 Http2::init()
@@ -843,6 +844,7 @@ Http2::init()
   REC_EstablishStaticConfigInt32U(write_buffer_block_size, "proxy.config.http2.write_buffer_block_size");
   REC_EstablishStaticConfigFloat(write_size_threshold, "proxy.config.http2.write_size_threshold");
   REC_EstablishStaticConfigInt32U(write_time_threshold, "proxy.config.http2.write_time_threshold");
+  REC_EstablishStaticConfigInt32U(buffer_water_mark, "proxy.config.http2.default_buffer_water_mark");
 
   // If any settings is broken, ATS should not start
   ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, max_concurrent_streams_in}));
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
index 9b7106393..dbc46744b 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -409,6 +409,7 @@ public:
   static uint32_t write_buffer_block_size;
   static float write_size_threshold;
   static uint32_t write_time_threshold;
+  static uint32_t buffer_water_mark;
 
   static void init();
 };
diff --git a/proxy/http2/Http2ClientSession.cc b/proxy/http2/Http2ClientSession.cc
index f3d6e4fac..415bec13b 100644
--- a/proxy/http2/Http2ClientSession.cc
+++ b/proxy/http2/Http2ClientSession.cc
@@ -116,10 +116,12 @@ Http2ClientSession::new_connection(NetVConnection *new_vc, MIOBuffer *iobuf, IOB
   this->_read_buffer_reader     = reader ? reader : this->read_buffer->alloc_reader();
 
   // This block size is the buffer size that we pass to SSLWriteBuffer
-  auto buffer_block_size_index = iobuffer_size_to_index(Http2::write_buffer_block_size, MAX_BUFFER_SIZE_INDEX);
-  this->write_buffer           = new_MIOBuffer(buffer_block_size_index);
-  this->_write_buffer_reader   = this->write_buffer->alloc_reader();
-  this->_write_size_threshold  = index_to_buffer_size(buffer_block_size_index) * Http2::write_size_threshold;
+  auto buffer_block_size_index   = iobuffer_size_to_index(Http2::write_buffer_block_size, MAX_BUFFER_SIZE_INDEX);
+  this->write_buffer             = new_MIOBuffer(buffer_block_size_index);
+  this->write_buffer->water_mark = Http2::buffer_water_mark;
+
+  this->_write_buffer_reader  = this->write_buffer->alloc_reader();
+  this->_write_size_threshold = index_to_buffer_size(buffer_block_size_index) * Http2::write_size_threshold;
 
   this->_handle_if_ssl(new_vc);
 
diff --git a/proxy/http2/Http2CommonSession.cc b/proxy/http2/Http2CommonSession.cc
index 55974b5b0..fb1cb20ae 100644
--- a/proxy/http2/Http2CommonSession.cc
+++ b/proxy/http2/Http2CommonSession.cc
@@ -413,6 +413,12 @@ Http2CommonSession::write_avail()
   return this->write_buffer->write_avail();
 }
 
+bool
+Http2CommonSession::is_write_high_water() const
+{
+  return this->write_buffer->high_water();
+}
+
 void
 Http2CommonSession::write_reenable()
 {
diff --git a/proxy/http2/Http2CommonSession.h b/proxy/http2/Http2CommonSession.h
index a9f156d77..c59a5d75e 100644
--- a/proxy/http2/Http2CommonSession.h
+++ b/proxy/http2/Http2CommonSession.h
@@ -103,6 +103,7 @@ public:
   void remember(const SourceLocation &location, int event, int reentrant = NO_REENTRANT);
 
   int64_t write_avail();
+  bool is_write_high_water() const;
 
   virtual ProxySession *get_proxy_session() = 0;
 
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 24b058485..648d9d9f0 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1655,12 +1655,6 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
     return Http2SendDataFrameResult::ERROR;
   }
 
-  if (this->session->write_avail() == 0) {
-    Http2StreamDebug(this->session, stream->get_id(), "Not write avail");
-    this->session->flush();
-    return Http2SendDataFrameResult::NOT_WRITE_AVAIL;
-  }
-
   // Select appropriate payload length
   if (resp_reader->is_read_avail_more_than(0)) {
     // We only need to check for window size when there is a payload
@@ -1679,6 +1673,12 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
     payload_length = 0;
   }
 
+  if (payload_length > 0 && this->session->is_write_high_water()) {
+    Http2StreamDebug(this->session, stream->get_id(), "Not write avail");
+    this->session->flush();
+    return Http2SendDataFrameResult::NOT_WRITE_AVAIL;
+  }
+
   stream->update_sent_count(payload_length);
 
   // Are we at the end?