-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
netty: create adaptive cumulator #9558
Merged
sergiitk
merged 6 commits into
grpc:master
from
sergiitk:netty-adaptive-cumulator-redo-4
Sep 28, 2022
Merged
netty: create adaptive cumulator #9558
sergiitk
merged 6 commits into
grpc:master
from
sergiitk:netty-adaptive-cumulator-redo-4
Sep 28, 2022
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
sergiitk
force-pushed
the
netty-adaptive-cumulator-redo-4
branch
from
September 21, 2022 17:42
ac6beff
to
47a89ed
Compare
sergiitk
force-pushed
the
netty-adaptive-cumulator-redo-4
branch
from
September 21, 2022 17:48
47a89ed
to
0bdaa6f
Compare
sergiitk
force-pushed
the
netty-adaptive-cumulator-redo-4
branch
3 times, most recently
from
September 21, 2022 18:04
04c60b7
to
528f823
Compare
sergiitk
force-pushed
the
netty-adaptive-cumulator-redo-4
branch
from
September 21, 2022 18:05
528f823
to
30d014e
Compare
ejona86
approved these changes
Sep 21, 2022
netty/src/main/java/io/grpc/netty/GrpcHttp2ConnectionHandler.java
Outdated
Show resolved
Hide resolved
sergiitk
changed the title
netty: create adaptive cumulator (redo attempt 4)
netty: Create adaptive cumulator
Sep 27, 2022
sergiitk
changed the title
netty: Create adaptive cumulator
netty: create adaptive cumulator
Sep 27, 2022
This was referenced Sep 27, 2022
larry-safran
pushed a commit
to larry-safran/grpc-java
that referenced
this pull request
Oct 6, 2022
Creates "Adaptive" cumulator: cumulate ByteBuf's by dynamically switching between merge and compose strategies. This cumulator applies a heuristic to make a decision whether to track a reference to the buffer with bytes received from the network stack in an array ("zero-copy"), or to merge into the last component (the tail) by performing a memory copy. It is necessary as a protection from a potential attack on the COMPOSITE_CUMULATOR. Consider a pathological case when an attacker sends TCP packages containing a single byte of data, and forcing the cumulator to track each one in a separate buffer. In this case we'll be paying a memory overhead for each buffer, as well as extra compute to read the cumulation. Implemented heuristic establishes a minimal threshold for the total size of the tail and incoming buffer, below which they are merged. The sum of the tail and the incoming buffer is used to avoid a case where attacker alternates the size of data packets to trick the cumulator into always selecting compose strategy. Merging strategy attempts to minimize unnecessary memory writes. When possible, it expands the tail capacity and only copies the incoming buffer into available memory. Otherwise, when both tail and the buffer must be copied, the tail is reallocated (or fully replaced) with a new buffer of exponentially increasing capacity (bounded to minComposeSize) to ensure runtime O(n^2) amortized to O(n). Note: this reintroduces grpc#7532, addressing the subtle issue (ref b/155940949) with `CompositeByteBuf.component()` indexes getting out of sync, which results in the merge operation producing broken buffers.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Creates "Adaptive" cumulator: cumulate ByteBuf's by dynamically switching between merge and compose strategies.
This cumulator applies a heuristic to make a decision whether to track a reference to the buffer with bytes received from the network stack in an array ("zero-copy"), or to merge into the last component (the tail) by performing a memory copy.
It is necessary as a protection from a potential attack on the COMPOSITE_CUMULATOR. Consider a pathological case when an attacker sends TCP packages containing a single byte of data, and forcing the cumulator to track each one in a separate buffer. In this case we'll be paying a memory overhead for each buffer, as well as extra compute to read the cumulation.
Implemented heuristic establishes a minimal threshold for the total size of the tail and incoming buffer, below which they are merged. The sum of the tail and the incoming buffer is used to avoid a case where attacker alternates the size of data packets to trick the cumulator into always selecting compose strategy.
Merging strategy attempts to minimize unnecessary memory writes. When possible, it expands the tail capacity and only copies the incoming buffer into available memory. Otherwise, when both tail and the buffer must be copied, the tail is reallocated (or fully replaced) with a new buffer of exponentially increasing capacity (bounded to minComposeSize) to ensure runtime O(n^2) amortized to O(n).
Note: this reintroduces #7532, addressing the subtle issue (ref b/155940949) with
CompositeByteBuf.component()
indexes getting out of sync, which results in the merge operation producing broken buffers.