[GitHub] [incubator-brpc] jamesge commented on a change in pull request #849: ignore flow control in h2 when sending first request

[GitBox <gi...@apache.org>]

jamesge commented on a change in pull request #849: ignore flow control in h2 when sending first request
URL: https://github.com/apache/incubator-brpc/pull/849#discussion_r305424299
 
 

 ##########
 File path: src/brpc/policy/http2_rpc_protocol.cpp
 ##########
 @@ -860,9 +866,28 @@ H2ParseResult H2Context::OnSettings(
         return MakeH2Message(NULL);
     }
     const int64_t old_stream_window_size = _remote_settings.stream_window_size;
-    if (!ParseH2Settings(&_remote_settings, it, frame_head.payload_size)) {
-        LOG(ERROR) << "Fail to parse from SETTINGS";
-        return MakeH2Error(H2_PROTOCOL_ERROR);
+    if (!_remote_settings_received) {
+        // To solve the problem that sender can't send large request before receving
+        // remote setting, the initial window size of stream/connection is set to
+        // MAX_WINDOW_SIZE(see constructor of H2Context).
+        // As a result, in the view of remote side, window size is 65535 by default so
+        // it may not send its stream size to sender, making stream size still be
+        // MAX_WINDOW_SIZE. In this case we need to revert this value to default.
+        H2Settings tmp_settings;
+        if (!ParseH2Settings(&tmp_settings, it, frame_head.payload_size)) {
+            LOG(ERROR) << "Fail to parse from SETTINGS";
+            return MakeH2Error(H2_PROTOCOL_ERROR);
+        }
+        _remote_settings = tmp_settings;
+        _remote_window_left.fetch_sub(
+                H2Settings::MAX_WINDOW_SIZE - H2Settings::DEFAULT_INITIAL_WINDOW_SIZE,
+                butil::memory_order_relaxed);
+        _remote_settings_received = true;
+    } else {
+        if (!ParseH2Settings(&_remote_settings, it, frame_head.payload_size)) {
+            LOG(ERROR) << "Fail to parse from SETTINGS";
+            return MakeH2Error(H2_PROTOCOL_ERROR);
+        }
     }
 
 Review comment:
   首先这段代码可以如下简化：
   ```
   if (!_remote_settings_received) {        
           _remote_settings.stream_window_size = H2Settings::DEFAULT_INITIAL_WINDOW_SIZE;
           _remote_window_left.fetch_sub(
                   H2Settings::MAX_WINDOW_SIZE - H2Settings::DEFAULT_INITIAL_WINDOW_SIZE,
                   butil::memory_order_relaxed);
           _remote_settings_received = true;
   }
   if (!ParseH2Settings(&_remote_settings, it, frame_head.payload_size)) {
           LOG(ERROR) << "Fail to parse from SETTINGS";
           return MakeH2Error(H2_PROTOCOL_ERROR);
   }
   ```
   
   其次目前的改法在WINDOWS_UPDATE过来前，还是有段时间_remote_window_left已经归位了，理论上做不到“只要server端connection-level window size设得很大就总能发送成功”，不过窗口比较小

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org