-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: add lz4
to SnapshotSegment::Headers
#6487
Conversation
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, pending @joshieDo @shekhirin
|
||
// Transaction and Receipt already have the compression scheme used natively in its encoding. | ||
// (zstd-dictionary) | ||
if matches!(segment, SnapshotSegment::Headers) { |
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.
nit: can add fn is_
for segment separately
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, nice. Love that it's a small change LOC-wise.
dd6aa63
to
613f319
Compare
Run this a couple times on both branches (with a cherry picked from #6608) and didn't see a difference so merging this. |
Adds compression to Header static file segments.
Holesky near tip
Before: 510MB
After: 352MB
On previous tests done before, there wasn't a noticeable impact on read performance, but will use flood to confirm it. So pending merge
Not adding for other segments, since they already leverage the codec compression (global zstd dictionary)