Description
The restart-required diff in
server/src/reload_diagnostics.rs
only compares a listener's name, address, protocol, TLS, and whether a
compression filter was added. It never looks at the rest of the Listener
fields, so changing any of these on a live reload is silently dropped with no
"requires restart" warning:
downstream_read_timeout_ms
max_connections
tcp_session_timeout_ms
tcp_max_duration_secs
cluster
upstream
All six are consumed when the Pingora service/handler is built at startup, not
per request. load_http_handler bakes downstream_read_timeout_ms and the
max_connections semaphore into the handler
(handler/mod.rs:117),
and TCP setup reads the timeouts plus upstream/cluster to group and build the
service
(tcp/tls_setup.rs:38).
A reload only swaps the Arc<ArcSwap<FilterPipeline>>, so none of these take
effect until a restart. The TCP grouping code already notes that differing
max_connections would be "silently discarded"
(tls_setup.rs:55).
The effect is misleading: an operator changes max_connections or a TCP
session timeout, sees the reload succeed, and assumes the new value is live when
the old one is still in force. Address/protocol/TLS/compression already warn
here; these fields should too.
Steps to reproduce
- Start the proxy with an HTTP listener that sets
max_connections: 1000 (or a
TCP listener with tcp_session_timeout_ms).
- Edit that value and save so the watcher reloads.
- The reload logs no restart-required warning, and the original limit/timeout
stays in effect.
Expected behavior
The reload diff warns that the changed field requires a restart, matching the
existing behavior for address/protocol/TLS/compression. The per-listener
detectors in reload_diagnostics.rs should also compare
downstream_read_timeout_ms, max_connections, tcp_session_timeout_ms,
tcp_max_duration_secs, cluster, and upstream.
Praxis version: main (v0.5.3 line)
Description
The restart-required diff in
server/src/reload_diagnostics.rsonly compares a listener's name, address, protocol, TLS, and whether a
compression filter was added. It never looks at the rest of the
Listenerfields, so changing any of these on a live reload is silently dropped with no
"requires restart" warning:
downstream_read_timeout_msmax_connectionstcp_session_timeout_mstcp_max_duration_secsclusterupstreamAll six are consumed when the Pingora service/handler is built at startup, not
per request.
load_http_handlerbakesdownstream_read_timeout_msand themax_connectionssemaphore into the handler(
handler/mod.rs:117),and TCP setup reads the timeouts plus
upstream/clusterto group and build theservice
(
tcp/tls_setup.rs:38).A reload only swaps the
Arc<ArcSwap<FilterPipeline>>, so none of these takeeffect until a restart. The TCP grouping code already notes that differing
max_connectionswould be "silently discarded"(
tls_setup.rs:55).The effect is misleading: an operator changes
max_connectionsor a TCPsession timeout, sees the reload succeed, and assumes the new value is live when
the old one is still in force. Address/protocol/TLS/compression already warn
here; these fields should too.
Steps to reproduce
max_connections: 1000(or aTCP listener with
tcp_session_timeout_ms).stays in effect.
Expected behavior
The reload diff warns that the changed field requires a restart, matching the
existing behavior for address/protocol/TLS/compression. The per-listener
detectors in
reload_diagnostics.rsshould also comparedownstream_read_timeout_ms,max_connections,tcp_session_timeout_ms,tcp_max_duration_secs,cluster, andupstream.Praxis version: main (v0.5.3 line)