zyearn 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_r305562300
##########
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:
1.这样简化是不是有个race：当client端第一个setting包收到后，将stream_window_size和_remote_window_left复原了，此时一个req的AppendAndDestroy正在调用，发现window太小，就发失败了。现在的pr代码可以避免”复原到初始“这个中间态。
2.从server的视角来说，client的default connection window
size一定是65536，所以一定是server端来负责在合适的时候发WINDOWS_UPDATE来保证client可以一直发。如果接收到setting后，并且WINDOWS_UPDATE一直没发过来，这时候发送失败是个预期的行为。如果server的connection
size很大，正确的实现是发完setting后立刻发WINDOWS_UPDATE（让client立刻感知），现在brpc server的代码就是这么实现的
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
