docs: fix the timeout behavior explanation #41783
Conversation
Signed-off-by: Rohit Agrawal <[email protected]>
|
Docs for this Pull Request will be rendered here: https://storage.googleapis.com/envoy-cncf-pr/41783/docs/index.html The docs are (re-)rendered each time the CI |
phlax
left a comment
phlax
left a comment
There was a problem hiding this comment.
tiny literal nit but otherwise looks like a really nice cleanup/addition
would be good to get sign off from @adisuissa for what is being said
|
@krajshiva can you PTAL, as you may have the most context |
Co-authored-by: phlax <[email protected]> Signed-off-by: Rohit Agrawal <[email protected]>
|
@adisuissa Is there anyone else who have expertise around this area? |
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
Thanks.
Left minor comments.
It would be good to have integration tests that validate the contents that is said here.
| otherwise specified is *1 hour*. To modify the idle timeout for downstream connections use the | ||
| :ref:`common_http_protocol_options | ||
| cluster HTTP connections. The idle timeout is defined as the period in which there are no active | ||
| requests or streams. When the idle timeout is reached, the connection will be closed. For HTTP/2 |
There was a problem hiding this comment.
Just to make sure, the timeout will occur even if the underlying TCP connection is kept alive (using keepalive)?
There was a problem hiding this comment.
I'll add clarification, Thanks!
| cluster HTTP connections. The idle timeout is defined as the period in which there are no active | ||
| requests or streams. When the idle timeout is reached, the connection will be closed. For HTTP/2 | ||
| downstream connections, a drain sequence will occur prior to closing the connection (see | ||
| :ref:`drain_timeout <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.drain_timeout>` |
There was a problem hiding this comment.
Please clarify if the drain_timeout is added on top of the idle_timeout.
There was a problem hiding this comment.
Per what I see the drain_timeout seems to start immediately when idle_timeout triggers and is not added on top. I'll rephrase the text.
Signed-off-by: Rohit Agrawal <[email protected]>
|
/retest |
4 participants
Description
We clarified the max_connection_duration behavior as part of this but this change was never done for the docs we have on "How do I configure timeouts?" here.
This PR is making the same changes to the main doc.
Commit Message: docs: fix the timeout behavior explanation
Additional Description: Fix the main timeout docs to align with the proto docs.
Risk Level: N/A
Testing: CI
Docs Changes: Added
Release Notes: N/A