Skip to content

http: max stream duration upstream support#10531

Merged
mattklein123 merged 33 commits intoenvoyproxy:masterfrom
Shikugawa:upstream-max-stream-duration
May 4, 2020
Merged

http: max stream duration upstream support#10531
mattklein123 merged 33 commits intoenvoyproxy:masterfrom
Shikugawa:upstream-max-stream-duration

Conversation

@Shikugawa
Copy link
Member

@Shikugawa Shikugawa commented Mar 26, 2020

Signed-off-by: shikugawa rei@tetrate.io

Commit Message: To resolve #10274, adding max stream duration for upstream connection.
Additional Description:
Risk Level: Low
Testing: Unit / Integration
Docs Changes: Required
Release Notes: Required
Fixes: #10274

Signed-off-by: shikugawa <rei@tetrate.io>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #10531 was opened by Shikugawa.

see: more, trace.

Signed-off-by: shikugawa <rei@tetrate.io>
@mattklein123 mattklein123 self-assigned this Mar 26, 2020
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks generally LGTM. Can you also make sure this is covered by an integration test?

/wait

@htuch
Copy link
Member

htuch commented Mar 27, 2020

/lgtm api

Signed-off-by: shikugawa <rei@tetrate.io>
@Shikugawa
Copy link
Member Author

It seems that disconnect with ActiveRequest is invalid because that emits request_encoder. It may lead to dangling references when clearing upstream requests.

Signed-off-by: shikugawa <rei@tetrate.io>
Signed-off-by: shikugawa <rei@tetrate.io>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks like this is on the right track. A few comments to work on. Thank you!

/wait

Signed-off-by: shikugawa <rei@tetrate.io>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few more comments, thank you!

/wait

Signed-off-by: shikugawa <rei@tetrate.io>
Signed-off-by: shikugawa <rei@tetrate.io>
@htuch
Copy link
Member

htuch commented Apr 7, 2020

Please merge master to pick up #10672. We no longer accept changes to v2 (without explicit exception), so any API modifications should happen in v3. If this PR is adding a new proto, please follow the updated instructions in https://github.com/envoyproxy/envoy/blob/master/api/STYLE.md#adding-an-extension-configuration-to-the-api.

Signed-off-by: shikugawa <rei@tetrate.io>
…tream-max-stream-duration

Signed-off-by: shikugawa <rei@tetrate.io>
Signed-off-by: shikugawa <rei@tetrate.io>
@mattklein123
Copy link
Member

Please merge master.

/wait

…tream-max-stream-duration

Signed-off-by: shikugawa <rei@tetrate.io>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm api

…tream-max-stream-duration

Signed-off-by: shikugawa <rei@tetrate.io>
Signed-off-by: shikugawa <rei@tetrate.io>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM with small comment.

/wait

Signed-off-by: shikugawa <rei@tetrate.io>
Signed-off-by: shikugawa <rei@tetrate.io>
mattklein123
mattklein123 previously approved these changes May 1, 2020
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@mattklein123 mattklein123 dismissed htuch’s stale review May 1, 2020 18:21

comments addressed

@mattklein123
Copy link
Member

@Shikugawa please cleanup the PR description per the new template. Thank you!

/wait-any

Shikugawa added 2 commits May 4, 2020 05:07
…tream-max-stream-duration

Signed-off-by: shikugawa <rei@tetrate.io>
Signed-off-by: shikugawa <rei@tetrate.io>
@repokitteh-read-only repokitteh-read-only bot removed the api label May 4, 2020
@mattklein123 mattklein123 merged commit 6151a69 into envoyproxy:master May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no stalebot Disables stalebot from closing an issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

http: add max stream time

4 participants