Copilot commented on code in PR #13695:
URL: https://github.com/apache/trafficserver/pull/13695#discussion_r4147771036
##########
src/proxy/http2/HTTP2.cc:
##########
@@ -45,6 +45,28 @@ struct Http2HeaderName {
static VersionConverter hvc;
+void
+establish_flow_control_policy(const char *name,
std::atomic<Http2FlowControlPolicy> &policy)
+{
+ auto update = [](const char *name, RecDataT type, RecData data, void
*cookie) -> int {
+ ink_assert(type == RECD_INT);
+ RecInt value = data.rec_int;
+
+ if (value < 0 || value > 2) {
+ Error("Invalid value for %s: %" PRId64, name, value);
+ value = 0;
+ }
+ static_cast<std::atomic<Http2FlowControlPolicy>
*>(cookie)->store(static_cast<Http2FlowControlPolicy>(value),
+
std::memory_order_relaxed);
+ return REC_ERR_OKAY;
+ };
Review Comment:
`ink_assert(type == RECD_INT);` in a config update callback can terminate
the process (or at least create an avoidable hard-failure) if the record type
is unexpected/mis-registered. Prefer a defensive runtime check (e.g., log +
return an error / ignore the update) instead of asserting in the callback path.
##########
src/proxy/http2/HTTP2.cc:
##########
@@ -45,6 +45,28 @@ struct Http2HeaderName {
static VersionConverter hvc;
+void
+establish_flow_control_policy(const char *name,
std::atomic<Http2FlowControlPolicy> &policy)
+{
+ auto update = [](const char *name, RecDataT type, RecData data, void
*cookie) -> int {
+ ink_assert(type == RECD_INT);
+ RecInt value = data.rec_int;
+
+ if (value < 0 || value > 2) {
+ Error("Invalid value for %s: %" PRId64, name, value);
Review Comment:
This uses the `PRId64` formatting macro; ensure this translation unit
includes the appropriate header (typically `<inttypes.h>` / `<cinttypes>`) so
builds don’t rely on indirect includes.
##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -5261,12 +5261,19 @@ HTTP/2 Configuration
a way that shares the window equally among all concurrent streams.
=====
===========================================================================================
+ Reloading this setting applies the new policy only to connections
initialized
+ after the update. Existing connections retain the policy selected when they
+ were initialized, including for streams opened after the reload. Close and
+ reopen a connection to use the new policy.
+
.. ts:cv:: CONFIG proxy.config.http2.flow_control.policy_out INT 0
:reloadable:
Specifies the mechanism |TS| uses to maintian flow control via the HTTP/2
Review Comment:
Correct spelling of 'maintian' to 'maintain'.
--
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]
