bneradt commented on code in PR #12531:
URL: https://github.com/apache/trafficserver/pull/12531#discussion_r2400679153
##########
src/proxy/http2/Http2ConnectionState.cc:
##########
@@ -1369,10 +1369,11 @@ Http2ConnectionState::send_connection_preface()
configured_settings.set(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS,
_adjust_concurrent_stream());
uint32_t const configured_initial_window_size =
this->_get_configured_receive_session_window_size();
- if (this->_has_dynamic_stream_window()) {
- // Since this is the beginning of the connection and there are no streams
- // yet, we can just set the stream window size to fill the entire session
- // window size.
+
+ // Since this is the beginning of the connection and there are no streams
+ // yet, we can just set the stream window size to fill the entire session
+ // window size.
Review Comment:
@randall , you've been very patient with us. Thank you for fixing this. I
have one more nit: for context, this comment really only applied to the dynamic
stream policy in which the stream size shifts dynamically as a function of the
session window and number of streams. The idea was that since there were
currently no streams at the beginning of the connection, we could make the next
stream use the entire session (connection) window if it wanted. This comment
doesn't really apply therefore to the static cases in which we configure the
stream and session windows separately.
tl;dr: Let's just remove the comment.
The code change looks good to me.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
