Skip to content

[chttp2] Limit request count before receiving settings ack#34638

Merged
ctiller merged 9 commits intogrpc:masterfrom
ctiller:barrier-chicken
Oct 10, 2023
Merged

[chttp2] Limit request count before receiving settings ack#34638
ctiller merged 9 commits intogrpc:masterfrom
ctiller:barrier-chicken

Conversation

@ctiller
Copy link
Copy Markdown
Member

@ctiller ctiller commented Oct 10, 2023

Previously chttp2 would allow infinite requests prior to a settings ack - as the agreed upon limit for requests in that state is infinite.
Instead, after MAX_CONCURRENT_STREAMS requests have been attempted, start blanket cancelling requests until the settings ack is received.
This can be done efficiently without allocating request state structures.

expiry: 2024/01/01
owner: ctiller@google.com
test_tags: []
- name: block_excessive_requests_before_settings_ack
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this need an entry in rollouts.yaml?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

@ctiller ctiller added the release notes: yes Indicates if PR needs to be in release notes label Oct 10, 2023
@ctiller ctiller merged commit 954b285 into grpc:master Oct 10, 2023
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Oct 11, 2023
ctiller added a commit to ctiller/grpc that referenced this pull request Oct 20, 2023
Previously chttp2 would allow infinite requests prior to a settings ack
- as the agreed upon limit for requests in that state is infinite.
Instead, after MAX_CONCURRENT_STREAMS requests have been attempted,
start blanket cancelling requests until the settings ack is received.
This can be done efficiently without allocating request state
structures.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bloat/low imported Specifies if the PR has been imported to the internal repository lang/core per-call-memory/neutral per-channel-memory/neutral release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants