-
Notifications
You must be signed in to change notification settings - Fork 328
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
feat: enable ArrowFlight compression #3403
Conversation
Signed-off-by: tison <wander4096@gmail.com>
.try_with_compression(Some(arrow::ipc::CompressionType::LZ4_FRAME)) | ||
.unwrap(); |
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.
Always Ok
because the default option has metadata_version = MetadataVersion::V5
.
But I'm a bit hesitate if we should hard code the compression type here, while I agree that we don't need to make it configurable. Fortunately, the receiver (reader) side would determinate the compression type and do the necessary decompresson work.
Signed-off-by: tison <wander4096@gmail.com>
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.
LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3403 +/- ##
==========================================
- Coverage 85.03% 84.63% -0.40%
==========================================
Files 889 889
Lines 146153 146326 +173
==========================================
- Hits 124281 123848 -433
- Misses 21872 22478 +606 |
I hereby agree to the terms of the GreptimeDB CLA
What's changed and what's your intention?
This closes #1445.
Although the issue suggests we enable compression for gRPC services also, I noticed that we are now initiating a lot of gRPC services. We may instead first sort out them so that we can determinate which services can turn on this feature and how their clients communicate with them.
Checklist
Refer to a related PR or issue link (optional)