Skip to content

Added max_requests_per_connection for downstream connection.#14936

Merged
mattklein123 merged 45 commits intoenvoyproxy:mainfrom
mk46:mx_rq_conn
Jul 19, 2021
Merged

Added max_requests_per_connection for downstream connection.#14936
mattklein123 merged 45 commits intoenvoyproxy:mainfrom
mk46:mx_rq_conn

Conversation

@mk46
Copy link
Copy Markdown
Contributor

@mk46 mk46 commented Feb 4, 2021 •
edited
Loading

Signed-off-by: Manish Kumar manish.kumar1@india.nec.com

Commit Message: Added max_requests_per_connection for downstream connection.
Additional Description: Added max_requests_per_connection for downstream connection. if not specified, there is no limit.
Risk Level:
Testing: added config test
Docs Changes: Done.
Release Notes: Done
Fixes #14531

Added max_requests_connection for downstream connection.
3906abe 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Feb 4, 2021

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
API shepherd assignee is @htuch
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #14936 was opened by mk46.

see: more, trace.

@ggreenway
Copy link
Copy Markdown
Member

ggreenway commented Feb 4, 2021

@htuch See my comment #14531 (comment).

Any advice from api-shepherds on which config approach is better?

htuch
htuch reviewed Feb 5, 2021
View reviewed changes
api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto Outdated

// Optional maximum requests for a downstream connection.
// If not specified, there is no limit.
google.protobuf.UInt32Value max_requests_connection = 43;
Copy link
Copy Markdown
Member

@htuch htuch Feb 5, 2021

Choose a reason for hiding this comment

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

@ggreenway @mk46 I'd vote to move this to HttpProtocolOptions and deprecate it in Cluster (with appropriate redirect in comments), it logically belongs there.

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 Feb 5, 2021

Choose a reason for hiding this comment

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

+1. cc @alyssawilk who has been working with this config a lot lately.

@htuch htuch added the waiting label Feb 7, 2021
Moved max_requests_connection to core.v3.HttpProtocolOptions.
ff52d2d 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
@mk46
Copy link
Copy Markdown
Contributor Author

mk46 commented Feb 11, 2021

PTAL

Fixed format and release docs.
5fa29e3 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
lambdai
lambdai reviewed Feb 11, 2021
View reviewed changes
Manish Kumar added 3 commits February 25, 2021 20:45
Undo deprecation.
163c635 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
Fixed CI.
3fecf7e 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
Merge remote-tracking branch 'upstream/main' into mx_rq_conn
68301da 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
@mk46 mk46 changed the title WIP: Added max_requests_connection for downstream connection. Added max_requests_connection for downstream connection. Feb 25, 2021
Fix.
236b9dc 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 27, 2021

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Mar 27, 2021
Manish Kumar added 2 commits April 1, 2021 19:41
fix
e46bd37 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
Merge remote-tracking branch 'upstream/main' into mx_rq_conn
6744578 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
Manish Kumar added 2 commits April 1, 2021 22:09
fix trailing whitespace
944e461 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
fix.
69b3990 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
@mk46
Copy link
Copy Markdown
Contributor Author

mk46 commented Apr 8, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Apr 8, 2021

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #14936 (comment) was created by @mk46.

see: more, trace.

@mk46 mk46 requested a review from htuch July 15, 2021 10:21
mattklein123
mattklein123 reviewed Jul 15, 2021
View reviewed changes
Manish Kumar and others added 2 commits July 15, 2021 23:19
@mattklein123
Updated comment
a1cd357 
Co-authored-by: Matt Klein <mattklein123@gmail.com>
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
Merge remote-tracking branch 'upstream/main' into mx_rq_conn
8a97141 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
fix rst
ee2bfd9 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
@mk46
Copy link
Copy Markdown
Contributor Author

mk46 commented Jul 15, 2021

@mattklein123 Sorry, due to DCO missing have to force push. Please take a look. Thanks!

@mk46 mk46 requested a review from mattklein123 July 15, 2021 18:09
@mk46
Copy link
Copy Markdown
Contributor Author

mk46 commented Jul 16, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Jul 16, 2021

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #14936 (comment) was created by @mk46.

see: more, trace.

fix comment
b34e753 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
mattklein123
mattklein123 approved these changes Jul 16, 2021
View reviewed changes
Copy link
Copy Markdown
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!

@htuch
Copy link
Copy Markdown
Member

htuch commented Jul 19, 2021

/lgtm api

@mattklein123 mattklein123 merged commit 364c821 into envoyproxy:main Jul 19, 2021
This was referenced Sep 23, 2021
Closed
Open
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Added max_requests_per_connection for downstream connection. (envoypr…
84a4f9b 
…oxy#14936)

Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
@sha-rath sha-rath mentioned this pull request Sep 30, 2021
Open
@Zsolt-LazarZsolt Zsolt-LazarZsolt mentioned this pull request Dec 15, 2021
Closed
jiangshantao-dbg pushed a commit to istio-mt/envoy that referenced this pull request May 16, 2022
feat: added max_requests_per_connection for downstream connection. (e…
5253896 
…nvoyproxy#14936)

Signed-off-by: jiangshantao <jiangshantao-dbg@qq.com>
jiangshantao-dbg pushed a commit to istio-mt/envoy that referenced this pull request May 16, 2022
@vjoke
feat: added max_requests_per_connection for downstream connection. (e…
e0924ef 
…nvoyproxy#14936) (#11)

Signed-off-by: jiangshantao <jiangshantao-dbg@qq.com>

Co-authored-by: jiangshantao <jiangshantao-dbg@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@alyssawilk alyssawilk Awaiting requested review from alyssawilk

@ggreenway ggreenway Awaiting requested review from ggreenway

@htuch htuch Awaiting requested review from htuch

+2 more reviewers

@antoniovicente antoniovicente antoniovicente left review comments

@lambdai lambdai lambdai requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mattklein123 mattklein123

@htuch htuch

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support for max_requests_connection in downstream connections

8 participants

@mk46 @ggreenway @hobbytp @alyssawilk @mattklein123 @htuch @lambdai @antoniovicente