New configuration parameter for HTTP2 maxFrameSize. #1253
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
|
|
gcjenkinson
force-pushed
the
gcj/http2-performance
branch
from
5c9a02a to
0faa956
Compare
glbrntt
requested changes
Sep 7, 2021
Sources/GRPC/Server.swift
Outdated
Comment on lines
459
to
463
| private func clamped<T>(_ value: T, lower: T, upper: T) -> T where T : Comparable { | ||
| return min(max(value, lower), upper) | ||
| } |
There was a problem hiding this comment.
This can be expressed a little more Swift-ly as:
extension Comparable {
fileprivate func clamped(to range: ClosedRange<Self>) -> Self {
return min(max(self, range.lowerBound), range.upperBound)
}
}
Sources/GRPC/Server.swift
Outdated
| /// inclusive. | ||
| public var httpMaxFrameSize: Int = 16384 { | ||
| didSet(httpMaxFrameSize) { | ||
| self.httpMaxFrameSize = clamped(httpMaxFrameSize, lower: 16_384, upper: 16_777_216) |
There was a problem hiding this comment.
The upper-bound is off-by-one here; the upper limit should be 2^24 - 1.
Sources/GRPC/Server.swift
Outdated
| @@ -332,6 +332,14 @@ extension Server { | |||
| } | |||
| } | |||
|
|
|||
| /// The HTTP/2 max frame size. Defaults to 16384. Must be value between 2^14 and 2^24-1 octets | |||
| /// inclusive. | |||
There was a problem hiding this comment.
Could you add a note saying that we clamp the value if it's outside the range?
gcjenkinson
force-pushed
the
gcj/http2-performance
branch
from
0faa956 to
4ceab7f
Compare
Motivation: In gRPC calls that process large payloads, the default settings of the HTTP2 maxFrameSize (16384) within swift-nio-http2 results in significant performance impact (for large payloads this performance impact has been demonstrated to result in timeouts where RPCs fail to complete within their assigned deadline). Allowing programmatic configuration of the HTTP2 maxFrameSize when the server is built allows the parameter to be optimised for the expected payload sizes. Modifications: Added new gRPC configuration parameter for the HTTP2 parameter maxFrameSize (which can be configured in the Server.Builder using the new method `withHTTPMaxFrameSize()`). Result: The Server.Builder can be used to configure the HTTP2 maxFrameSize parameter as: Server.insecure(group: serverGroup).withHTTPMaxFrameSize(1024 * 1024)...
gcjenkinson
force-pushed
the
gcj/http2-performance
branch
from
4ceab7f to
dc40e0b
Compare
glbrntt
approved these changes
Sep 8, 2021
glbrntt
added a commit
to glbrntt/grpc-swift
that referenced
this pull request
Sep 8, 2021
Motivation: Having more control over the configuration of the http/2 layer is useful for certain performance sensitive situations. We recently exposed the max frame size setting on the server (grpc#1253); we should do the same on the client. Modifications: - Add configuration for the max frame size to the client - Fix the clamping of the max frame size on the server (this was missed when reviewing grpc#1253) - Add tests to validate that the configured values are correctly clamped - Send `SETTINGS_INITIAL_WINDOW_SIZE` in the initial SETTINGS frame set to the value of the target window size. Result: Users have more control over HTTP/2 settings.
glbrntt
added a commit
that referenced
this pull request
Sep 8, 2021
Motivation: Having more control over the configuration of the http/2 layer is useful for certain performance sensitive situations. We recently exposed the max frame size setting on the server (#1253); we should do the same on the client. Modifications: - Add configuration for the max frame size to the client - Fix the clamping of the max frame size on the server (this was missed when reviewing #1253) - Add tests to validate that the configured values are correctly clamped - Send `SETTINGS_INITIAL_WINDOW_SIZE` in the initial SETTINGS frame set to the value of the target window size. Result: Users have more control over HTTP/2 settings.
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.
Motivation:
In gRPC calls that process large payloads, the default settings of the
HTTP2 maxFrameSize (16384) within swift-nio-http2 results in significant
performance impact (for large payloads this performance impact has been
demonstrated to result in timeouts where RPCs fail to complete within
their assigned deadline).
Allowing programmatic configuration of the HTTP2 maxFrameSize when the
server is built allows the parameter to be optimised for the expected
payload sizes.
Modifications:
Added new gRPC configuration parameter for the HTTP2 parameter
maxFrameSize (which can be configured in the Server.Builder using the
new method
withHTTPMaxFrameSize()).Result:
The Server.Builder can be used to configure the HTTP2 maxFrameSize
parameter as:
Server.insecure(group: serverGroup).withHTTPMaxFrameSize(1024 * 1024)...