-
Notifications
You must be signed in to change notification settings - Fork 6.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
compression_per_level
should be used for flush and changeable
#9658
Conversation
@@ -271,6 +270,7 @@ struct MutableCFOptions { | |||
Temperature bottommost_temperature; | |||
|
|||
uint64_t sample_for_compression; | |||
std::vector<CompressionType> compression_per_level; |
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.
Minor nit: Can we try to keep the order of the structure elements the same between the ColumnFamilyOptions and the Mutable/Immutable ones? It makes it easier for comparison/checking purposes.
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.
MutableCFOptions
groups the options by memtable, compaction, blob, misc, we're putting it to misc group with other compression options:
https://github.com/facebook/rocksdb/blob/6cb7d64259110345a86ad48252df8dacd8c623cd/options/cf_options.h#L246-L260
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.
74170b5
to
6fd3504
Compare
compression_per_level
should be used for flush and dynamic changeable
compression_per_level
should be used for flush and dynamic changeablecompression_per_level
should be used for flush and changeable
@jay-zhuang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
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.
Nice! LGTM
compression_per_level
dynamical changeable withSetOptions
;compression_per_level
is not used for flush;Test Plan: CI