dolysis
/
rocksdb
mirror of https://github.com/facebook/rocksdb.git
3
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Persist table options use_delta_encoding in options file (#11987) 

Summary:
This option is used for encoding keys in block based table files. It has been having a default true value since its introduction.

Users may not notice this option is not persisted in options file unless they are explicitly setting it to false. If the users expect `Iterator::GetProperty("rocksdb.iterator.is-key-pinned")` to return 1 when setting `ReadOptions.pin_data = true`, they should have noticed loading options file won't work and have work around for this by always explicitly set this option to false for opening DB. This change won't impact those users except that now they can remove their work around. If the users are not relying on key pinning behavior at all and as a result didn't notice the option is not persisted, this change shouldn't have any visible behavior impact either.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/11987

Reviewed By: hx235

Differential Revision: D54093238

Pulled By: jowlyzhang

fbshipit-source-id: 256a3348c44cf91349034d1f6e242c437b32b9a5
This commit is contained in:
Yu Zhang 2024-02-23 14:13:28 -08:00 committed by Facebook GitHub Bot
parent f300438c20
commit 2940acac00
3 changed files with 6 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
java/src/test/java/org/rocksdb/OptionsUtilTest.java
View File

@ -358,6 +358,7 @@ public class OptionsUtilTest {
assertThat(actual.metadataBlockSize()).isEqualTo(expected.metadataBlockSize());
assertThat(actual.partitionFilters()).isEqualTo(expected.partitionFilters());
assertThat(actual.optimizeFiltersForMemory()).isEqualTo(expected.optimizeFiltersForMemory());
assertThat(actual.useDeltaEncoding()).isEqualTo(expected.useDeltaEncoding());
assertThat(actual.wholeKeyFiltering()).isEqualTo(expected.wholeKeyFiltering());
assertThat(actual.verifyCompression()).isEqualTo(expected.verifyCompression());
assertThat(actual.readAmpBytesPerBit()).isEqualTo(expected.readAmpBytesPerBit());
@ -370,9 +371,5 @@ public class OptionsUtilTest {
} else {
assertThat(expected.filterPolicy().equals(actual.filterPolicy()));
}
// not currently persisted - always true when read from options
// this test will fail, and need repaired, if and when "useDeltaEncoding" is persisted.
assertThat(actual.useDeltaEncoding()).isEqualTo(true);
}
}

3
options/options_settable_test.cc
View File

@ -157,8 +157,6 @@ TEST_F(OptionsSettableTest, BlockBasedTableOptionsAllFieldsSettable) {
// GetBlockBasedTableOptionsFromString().
bbto = new (bbto_ptr) BlockBasedTableOptions();
FillWithSpecialChar(bbto_ptr, sizeof(BlockBasedTableOptions), kBbtoExcluded);
// This option is not setable:
bbto->use_delta_encoding = true;
char* new_bbto_ptr = new char[sizeof(BlockBasedTableOptions)];
BlockBasedTableOptions* new_bbto =
@ -191,6 +189,7 @@ TEST_F(OptionsSettableTest, BlockBasedTableOptionsAllFieldsSettable) {
"metadata_block_size=1024;"
"partition_filters=false;"
"optimize_filters_for_memory=true;"
"use_delta_encoding=true;"
"index_block_restart_interval=4;"
"filter_policy=bloomfilter:4:true;whole_key_filtering=1;detect_filter_"
"construct_corruption=false;"

7
table/block_based/block_based_table_factory.cc
View File

@ -308,9 +308,10 @@ static std::unordered_map<std::string, OptionTypeInfo>
{offsetof(struct BlockBasedTableOptions, optimize_filters_for_memory),
OptionType::kBoolean, OptionVerificationType::kNormal,
OptionTypeFlags::kNone}},
// TODO "use_delta_encoding" has not been persisted -
// this may have been an omission, but changing this now might be a
// breaker
{"use_delta_encoding",
{offsetof(struct BlockBasedTableOptions, use_delta_encoding),
OptionType::kBoolean, OptionVerificationType::kNormal,
OptionTypeFlags::kNone}},
{"filter_policy",
OptionTypeInfo::AsCustomSharedPtr<const FilterPolicy>(
offsetof(struct BlockBasedTableOptions, filter_policy),