-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
engine: per try idle timeout #1805
Conversation
Signed-off-by: Jose Nino <jnino@lyft.com>
#1804 is needed for this to go green |
380999c
to
77614fb
Compare
Signed-off-by: Jose Nino <jnino@lyft.com>
…try-idle Signed-off-by: Jose Nino <jnino@lyft.com>
@@ -174,7 +174,7 @@ Specify the rate at which Envoy Mobile should flush its queued stats. | |||
|
|||
Specifies the length of time a stream should wait without a headers or data event before timing out. | |||
Defaults to 15 seconds. | |||
See `the Envoy docs <https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-field-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager-stream-idle-timeout>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this deliberate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure why @buildbreaker updated. But still seems to generate correctly.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Specifies the length of time a retry (including the initial attempt) should wait without a headers | ||
or data event before timing out. Defaults to 15 seconds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this apply to trailers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this should say "wait for bytes". That would be more comprehensive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@buildbreaker let me fix this in a subsequent PR so we don't have to run CI on this again.
@buildbreaker going to merge so you and @rebello95 can wire up and we can test. |
sounds good |
Description: expose the new per try idle timeout via the engine builder.
Risk Level: low
Testing: unit test
Docs Changes: added