fix: Add gRPC timeout and keepalive to RemoteRegistry - #6698
Conversation
Signed-off-by: Pelong <arthur.pelong@ecl22.ec-lyon.fr>
Signed-off-by: Pelong <arthur.pelong@ecl22.ec-lyon.fr>
franciscojavierarceo
left a comment
franciscojavierarceo
left a comment
There was a problem hiding this comment.
The new tuning fields are unconstrained Optional[int] values. timeout=0 or a negative keepalive interval/timeout is accepted by RemoteRegistryConfig and only fails later when gRPC constructs or uses the channel, turning a config typo into a runtime startup failure. We should validate positive values at config load (and add tests for zero/negative inputs); if None is intended to mean no deadline, the docs should also avoid calling this a default timeout.
Signed-off-by: Arthur Pelong <arthurpelong@gmail.com>
|
Thanks for the feedback! I've added a Pydantic |
| Required when the connection address differs from the service hostname, | ||
| e.g. when connecting through a tunnel or proxy for local development. """ | ||
|
|
||
| timeout: Optional[int] = None |
There was a problem hiding this comment.
should it be Optional[float] (or Optional[Union[int, float]]) ?
| """ int: Period in milliseconds after which a keepalive ping is sent on the transport. Must be strictly positive. """ | ||
|
|
||
| keepalive_timeout_ms: Optional[int] = None | ||
| """ int: Timeout in milliseconds for keepalive ping acknowledgement. Must be strictly positive. """ |
There was a problem hiding this comment.
should there be keepalive_permit_without_calls also ?
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6698 +/- ##
==========================================
+ Coverage 46.77% 46.79% +0.02%
==========================================
Files 414 414
Lines 50191 50211 +20
Branches 7181 7185 +4
==========================================
+ Hits 23475 23496 +21
+ Misses 25077 25076 -1
Partials 1639 1639
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
5 participants
What this PR does / why we need it:
Configures a default deadline (timeout) and transport keepalive options (
keepalive_time_ms,keepalive_timeout_ms) for the client-side gRPC channel inRemoteRegistry.Without these configurations, network drops or silent firewall cuts (blackholes) after a TCP connection is established cause all registry calls to hang indefinitely in
epoll_waitwithout raising any errors or timeouts.Which issue(s) this PR fixes:
Fixes #6665
Checks
git commit -s)Testing Strategy
Misc
NONE