Allows configuring the initial HTTP/2 settings #33777
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🙈 The PR is closed and the preview is expired. |
This comment has been minimized.
This comment has been minimized.
|
The Maven test failures seem suspicious... |
cescoffier
force-pushed
the
add-missing-http2-settings
branch
from
02c09bc to
1a0bdae
Compare
|
@geoand yes and no, they got fixed in another PR about the double Kubernetes service ports. I just based my PR on a broken version. I've rebased, so, it should be fine. |
|
👌 |
Failing Jobs - Building 1a0bdae
Full information is available in the Build summary check run. Failures⚙️ Native Tests - Amazon #- Failing: integration-tests/amazon-lambda-http
📦 integration-tests/amazon-lambda-http✖ |
geoand
reviewed
Jun 6, 2023
Comment on lines
+50
to
+87
| /** | ||
| * Set the SETTINGS_HEADER_TABLE_SIZE HTTP/2 setting. | ||
| * <p> | ||
| * Allows the sender to inform the remote endpoint of the maximum size of the header compression table used to decode | ||
| * header blocks, in octets. The encoder can select any size equal to or less than this value by using signaling | ||
| * specific to the header compression format inside a header block. | ||
| * The initial value is {@code 4,096} octets. | ||
| */ | ||
| @ConfigItem | ||
| public OptionalLong headerTableSize; | ||
|
|
||
| /** | ||
| * Set SETTINGS_MAX_CONCURRENT_STREAMS HTTP/2 setting. | ||
| * <p> | ||
| * Indicates the maximum number of concurrent streams that the sender will allow. This limit is directional: it | ||
| * applies to the number of streams that the sender permits the receiver to create. Initially, there is no limit to | ||
| * this value. It is recommended that this value be no smaller than 100, to not unnecessarily limit parallelism. | ||
| */ | ||
| @ConfigItem | ||
| public OptionalLong maxConcurrentStreams; | ||
|
|
||
| /** | ||
| * Set the SETTINGS_MAX_FRAME_SIZE HTTP/2 setting. | ||
| * Indicates the size of the largest frame payload that the sender is willing to receive, in octets. | ||
| * The initial value is {@code 2^14} (16,384) octets. | ||
| */ | ||
| @ConfigItem | ||
| public OptionalInt maxFrameSize; | ||
|
|
||
| /** | ||
| * Set the SETTINGS_MAX_HEADER_LIST_SIZE HTTP/2 setting. | ||
| * This advisory setting informs a peer of the maximum size of header list that the sender is prepared to accept, | ||
| * in octets. The value is based on the uncompressed size of header fields, including the length of the name and | ||
| * value in octets plus an overhead of 32 octets for each header field. | ||
| * The default value is {@code 8192} | ||
| */ | ||
| @ConfigItem | ||
| public OptionalLong maxHeaderListSize; |
There was a problem hiding this comment.
Just a thought: Should these be under a dedicated http2 group?
There was a problem hiding this comment.
I was wondering the same, but because these are limits, it's a bit weird to have them separately like:
- quarkus.http.limits for http/1 and http/1.1
- quarkus.http.http2 for http/2
But I don't have a strong opinion.
There was a problem hiding this comment.
But I don't have a strong opinion.
Me neither, so let's just leave it as is
geoand
approved these changes
Jun 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #33692