-
Couldn't load subscription status.
- Fork 5.5k
chore: Add compression support for reactor-netty http2 client #26381
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
base: master
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR implements optional zstd-based compression for HTTP2 task update requests and responses, adds configurable compression and TCP buffer settings in ReactorNettyHttpClientConfig, applies these settings in ReactorNettyHttpClient initialization (enabling HTTP compression and buffer options), injects per-request logic to compress POST bodies when thresholds are met, and adds the zstd-jni dependency. Sequence diagram for POST request compression in ReactorNettyHttpClientsequenceDiagram
participant Coordinator
participant "ReactorNettyHttpClient"
participant "ZstdOutputStreamNoFinalizer"
participant "HTTP2 Worker"
Coordinator->>"ReactorNettyHttpClient": send POST request (body)
alt Compression enabled & body >= threshold
"ReactorNettyHttpClient"->>"ZstdOutputStreamNoFinalizer": compress body
"ZstdOutputStreamNoFinalizer"-->>"ReactorNettyHttpClient": compressed body
"ReactorNettyHttpClient"->>"HTTP2 Worker": POST (compressed body, header Content-Encoding: zstd)
else Compression not enabled or body < threshold
"ReactorNettyHttpClient"->>"HTTP2 Worker": POST (original body)
end
Entity relationship diagram for new configuration properties in ReactorNettyHttpClientConfigerDiagram
REACTOR_NETTY_HTTP_CLIENT_CONFIG {
boolean dataCompressionEnabled
int dataCompressionThreshold
double useCompressedDataThreshold
int tcpBufferSize
}
Class diagram for updated ReactorNettyHttpClientConfig and ReactorNettyHttpClientclassDiagram
class ReactorNettyHttpClientConfig {
+boolean dataCompressionEnabled
+DataSize dataCompressionThreshold
+double useCompressedDataThreshold
+DataSize tcpBufferSize
+setDataCompressionEnabled(boolean)
+isDataCompressionEnabled() boolean
+getUseCompressedDataThreshold() double
+setUseCompressedDataThreshold(double) ReactorNettyHttpClientConfig
+getTcpBufferSize() int
+setTcpBufferSize(DataSize) ReactorNettyHttpClientConfig
+getDataCompressionThreshold() int
+setDataCompressionThreshold(DataSize) ReactorNettyHttpClientConfig
}
class ReactorNettyHttpClient {
-boolean isDataCompressionEnabled
-int dataCompressionThreshold
-double useCompressedDataThreshold
+ReactorNettyHttpClient(ReactorNettyHttpClientConfig, HttpClientConnectionPoolStats, HttpClientStats)
}
ReactorNettyHttpClientConfig <|-- ReactorNettyHttpClient
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `presto-main/src/main/java/com/facebook/presto/server/remotetask/ReactorNettyHttpClient.java:250` </location>
<code_context>
+ }
+
+ byte[] compressedBytes = baos.toByteArray();
+ double compressionRatio = (double) (postBytes.length - compressedBytes.length) / postBytes.length;
+ if (compressionRatio >= useCompressedDataThreshold) {
+ bodyToSend = compressedBytes;
</code_context>
<issue_to_address>
**issue (bug_risk):** Compression ratio calculation may be negative if compressedBytes is larger than postBytes.
Negative compressionRatio values may cause compressed data to be used when it is actually larger than the original. Ensure the ratio is non-negative, for example by using Math.max(0, compressionRatio).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
|
|
||
| byte[] compressedBytes = baos.toByteArray(); | ||
| double compressionRatio = (double) (postBytes.length - compressedBytes.length) / postBytes.length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Compression ratio calculation may be negative if compressedBytes is larger than postBytes.
Negative compressionRatio values may cause compressed data to be used when it is actually larger than the original. Ensure the ratio is non-negative, for example by using Math.max(0, compressionRatio).
Description
Motivation and Context
Impact
Test Plan
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.