bneradt commented on PR #13695: URL: https://github.com/apache/trafficserver/pull/13695#issuecomment-5898256555
@maskit, in response to [your comment](https://github.com/apache/trafficserver/pull/13695#issuecomment-5898148628): You’re right that this change allows existing H2 connections to observe the updated policy. The callback updates the global atomic, and `_get_configured_flow_control_policy()` reads it when needed, including when creating streams and replenishing receive windows. It doesn’t snapshot the policy at connection initialization. I checked other reloadable H2 settings, and there are examples of both behaviors: | Setting (`proxy.config.http2.*`) | How it is used | |---|---| | `max_ping_frames_per_minute`, `max_settings_frames_per_minute`, and other frame-rate limits | Copied into connection members in `Http2ConnectionState::init()`. Existing connections retain their values. | | `max_settings_per_frame`, `max_settings_per_minute` | Read directly from globals when processing SETTINGS frames, so existing connections observe updates. | | `initial_window_size_in/out` | Read from globals when calculating receive-window targets, including during window replenishment on existing connections. | The [per-connection copies are here](https://github.com/apache/trafficserver/blob/f09c345910007db50699ec3cbc4cf8251f36ada2/src/proxy/http2/Http2ConnectionState.cc#L1355-L1360), while the [SETTINGS limits are read here](https://github.com/apache/trafficserver/blob/f09c345910007db50699ec3cbc4cf8251f36ada2/src/proxy/http2/Http2ConnectionState.cc#L776-L818). For flow control, [window replenishment](https://github.com/apache/trafficserver/blob/f09c345910007db50699ec3cbc4cf8251f36ada2/src/proxy/http2/Http2ConnectionState.cc#L2009-L2020) recalculates the target using the current policy and initial window configuration. So the current approach follows the existing dynamic reads for the related window settings. That said, this doesn’t by itself establish that changing policies mid-connection is desirable. The regression test opens fresh connections after each update; it doesn’t exercise policy transitions on an existing connection. If you prefer the policy to remain fixed for each connection, we can snapshot it in `Http2ConnectionState::init()` and have reloads affect only new connections. Do you think that would be the better behavior here? -- 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]
