mobile: add knob for h3 keepalive #36646
Conversation
Signed-off-by: Dan Zhang <danzh@google.com>
danzh2010
requested review from
RyanTheOptimist,
alyssawilk,
abeyad and
fredyw
as code owners
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
mobile/library/cc/engine_builder.cc
Outdated
| ->mutable_http3_protocol_options() | ||
| ->mutable_quic_protocol_options(); | ||
| quic_protocol_options->mutable_connection_keepalive()->mutable_initial_interval()->set_nanos( | ||
| keepalive_initial_interval_ms_ * 1000 * 1000); |
There was a problem hiding this comment.
since the default value is 0 in EngineBuilder, this will disable keep alive entirely by default:
.There was a problem hiding this comment.
Actually, it looks like the default is to disable keep alive anyway:
.I thought we had keep alive but it was some larger value like 15s?
There was a problem hiding this comment.
Your first link:
For a client connection, if :ref:`max_interval <envoy_v3_api_field_config.core.v3.QuicKeepAliveSettings.max_interval>` is also zero, do not keepalive, otherwise use max_interval or QUICHE default to probe all the time.
There was a problem hiding this comment.
ah yes, sorry I missed that, thanks!
There was a problem hiding this comment.
so this appears to pass but I think it's because we disable proto gen validate for Envoy. Aren't you setting it explicitly to zero while retaining a config setting that it is supposed to be GTE some number of nanos? Should the restriction be removed entirely or should we not set it to zero for E-M?
There was a problem hiding this comment.
I changed it to check non-zero before setting the protobuf.
|
test failures look legit though, e.g. https://github.com/envoyproxy/envoy/actions/runs/11369001000/job/31625515074 |
|
no relevant owners for "mobile" |
|
/assign @alyssawilk since it contains core code, Alyssa should merge |
|
/retest |
Signed-off-by: Dan Zhang <danzh@google.com>
|
/retest |
Commit Message: add a config knob to Java, Kotlin and C++ engines to set initial interval of QUIC keepalive probing.
Additional Description: also adjust the validation rule of
initial_intervalto be larger than 1ms instead of 1s and fix contradicting documentation.Risk Level: low, interface change
Testing: unit test
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A