-
Notifications
You must be signed in to change notification settings - Fork 25.5k
Remove zstd feature flag for index codec best compression. #112665
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
Remove zstd feature flag for index codec best compression. #112665
Conversation
Hi @martijnvg, I've created a changelog YAML for you. Note that since this PR is labelled |
Note that release tests fail, because of esql tests that assume a feature flag is active. |
Hi @martijnvg, I've updated the changelog YAML for you. Note that since this PR is labelled |
Pinging @elastic/es-docs (Team:Docs) |
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
Hi @martijnvg, I've updated the changelog YAML for you. Note that since this PR is labelled |
Hi @martijnvg, I've updated the changelog YAML for you. Note that since this PR is labelled |
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.
Woohoo!
indexing throughput (~2%). | ||
have shown that `best_compression` gives up to ~28% lower storage usage and | ||
better indexing throughput (up to ~10%) in the most ideal scenario compared | ||
to `default` while affecting get by id latencies up to 10%. The higher get |
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.
only 10% sounds surprising?
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.
Right, maybe too optimistic. Based on tsdb get by id visualizations, this is a higher (at least 12%). Let me update this as well to be more realistic.
the most ideal scenario compared to `default` while only minimally affecting | ||
indexing throughput (~2%). | ||
have shown that `best_compression` gives up to ~28% lower storage usage and | ||
better indexing throughput (up to ~10%) in the most ideal scenario compared |
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.
I imagine that it can be faster at indexing than default
in some cases via more batching, but also slower in other cases e.g. when index sorting is involved as some merging optimizations that copy data directly would no longer be applicable. I would rather rephase to suggest similar indexing rates, occasionally a bit slower or a bit faster depending on other options configured on the index?
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.
👍 - I will reword this. This is based on comparing elastic/logs track without logsdb with default codec (lz4) codec to elastic/logs track without logsdb with best_compression (zstd)
docs/changelog/112665.yaml
Outdated
Before DEFLATE compression was used to compress stored fields in indices with index.codec index setting set to | ||
best_compression, with this change ZStandard is used as compression algorithm to stored fields for indices with | ||
index.codec index setting set to best_compression. Experiments have shown that ZStandard offers lower storage usage | ||
(upto ~12%) and higher indexing throughput (upto 14%) compared to DEFLATE. |
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.
should we also update the note on higher indexing throughput here?
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.
yes, I reworded this to: 58e4898
run elasticsearch-ci/release-tests |
Release build failed with: |
…12665) ZStandard was added via elastic#103374 a few months ago to snapshot builds of Elasticsearch only and benchmark results have shown that using zstd is a better trade off compared to deflate for when index.codec is set to best_compression. This change removes the feature flag for ZStandard stored field compression for indices with index.codec set to best_compression.
💚 Backport successful
|
…2665) (#112857) * Remove zstd feature flag for index codec best compression. (#112665) ZStandard was added via #103374 a few months ago to snapshot builds of Elasticsearch only and benchmark results have shown that using zstd is a better trade off compared to deflate for when index.codec is set to best_compression. This change removes the feature flag for ZStandard stored field compression for indices with index.codec set to best_compression. * Update docs/changelog/112857.yaml --------- Co-authored-by: Elastic Machine <[email protected]>
ZStandard was added via #103374 a few months ago to snapshot builds of Elasticsearch only and benchmark results have shown that using zstd is a better trade off compared to deflate for when
index.codec
is set tobest_compression
.This change removes the feature flag for ZStandard stored field compression for indices with
index.codec
set tobest_compression
.