bneradt commented on code in PR #12531:
URL: https://github.com/apache/trafficserver/pull/12531#discussion_r2395566174
##########
src/proxy/http2/Http2ConnectionState.cc:
##########
@@ -1368,14 +1368,18 @@ 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();
+ uint32_t configured_initial_window_size;
if (this->_has_dynamic_stream_window()) {
Review Comment:
@randall : your patch looks fine in its intention.
I also think that @maskit's tweak on top of your patch is a correct and good
improvement. It's been 3 years, but I believe I was simply trying to avoid
unecessary use of `HTTP2_SETTINGS_INITIAL_WINDOW_SIZE` and for some reason
thought it was only needed for dynamic streams. But clearly that's not the
case. Let's just send the `HTTP2_SETTINGS_INITIAL_WINDOW_SIZE` regardless of
dynamic or not.
However, we maybe should not send the setting if the configured value is
already the default `HTTP2_INITIAL_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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
