Skip to content

netty: create adaptive cumulator - #9558

Merged
sergiitk merged 6 commits into
grpc:masterfrom
sergiitk:netty-adaptive-cumulator-redo-4
Sep 28, 2022
Merged

sergiitk merged 6 commits into
grpc:masterfrom
sergiitk:netty-adaptive-cumulator-redo-4

Conversation

@sergiitk

@sergiitk sergiitk commented Sep 21, 2022 •

Copy link
Copy Markdown
Member

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.

@sergiitk
sergiitk force-pushed the netty-adaptive-cumulator-redo-4 branch from ac6beff to 47a89ed Compare September 21, 2022 17:42
@sergiitk
sergiitk force-pushed the netty-adaptive-cumulator-redo-4 branch from 47a89ed to 0bdaa6f Compare September 21, 2022 17:48
@sergiitk
sergiitk force-pushed the netty-adaptive-cumulator-redo-4 branch 3 times, most recently from 04c60b7 to 528f823 Compare September 21, 2022 18:04
@sergiitk
sergiitk force-pushed the netty-adaptive-cumulator-redo-4 branch from 528f823 to 30d014e Compare September 21, 2022 18:05
@sergiitk
sergiitk marked this pull request as ready for review September 21, 2022 20:15
@sergiitk
sergiitk requested a review from ejona86 September 21, 2022 20:15
Comment thread netty/src/main/java/io/grpc/netty/GrpcHttp2ConnectionHandler.java Outdated
@sergiitk sergiitk changed the title netty: create adaptive cumulator (redo attempt 4) netty: Create adaptive cumulator Sep 27, 2022
@sergiitk sergiitk changed the title netty: Create adaptive cumulator netty: create adaptive cumulator Sep 27, 2022
@sergiitk
sergiitk enabled auto-merge (squash) September 28, 2022 01:57
@sergiitk
sergiitk merged commit eaf8b28 into grpc:master Sep 28, 2022
@sergiitk
sergiitk deleted the netty-adaptive-cumulator-redo-4 branch September 28, 2022 17:20
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.
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Dec 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants