Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove deprecated block-based filter #10184

Closed
wants to merge 3 commits into from
Closed

Remove deprecated block-based filter #10184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
367966e
Remove deprecated block-based filter
pdillinger Jun 16, 2022
1a938ee
Some enhancements, HISTORY
pdillinger Jun 16, 2022
02c7f66
Merge remote-tracking branch 'origin/main' into remove_bbf
pdillinger Jun 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,6 @@ set(SOURCES
table/adaptive/adaptive_table_factory.cc
table/block_based/binary_search_index_reader.cc
table/block_based/block.cc
table/block_based/block_based_filter_block.cc
table/block_based/block_based_table_builder.cc
table/block_based/block_based_table_factory.cc
table/block_based/block_based_table_iterator.cc
Expand Down Expand Up @@ -1321,7 +1320,6 @@ if(WITH_TESTS)
options/customizable_test.cc
options/options_settable_test.cc
options/options_test.cc
table/block_based/block_based_filter_block_test.cc
table/block_based/block_based_table_reader_test.cc
table/block_based/block_test.cc
table/block_based/data_block_hash_index_test.cc
Expand Down
1 change: 1 addition & 0 deletions HISTORY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

### Behavior changes
* DB::Open(), DB::OpenAsSecondary() will fail if a Logger cannot be created (#9984)
* Removed support for reading Bloom filters using obsolete block-based filter format. (Support for writing such filters was dropped in 7.0.) For good read performance on old DBs using these filters, a full compaction is required.

## 7.3.0 (05/20/2022)
### Bug Fixes
Expand Down
3 changes: 0 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1594,9 +1594,6 @@ random_access_file_reader_test: $(OBJ_DIR)/file/random_access_file_reader_test.o
file_reader_writer_test: $(OBJ_DIR)/util/file_reader_writer_test.o $(TEST_LIBRARY) $(LIBRARY)
$(AM_LINK)

block_based_filter_block_test: $(OBJ_DIR)/table/block_based/block_based_filter_block_test.o $(TEST_LIBRARY) $(LIBRARY)
$(AM_LINK)

block_based_table_reader_test: table/block_based/block_based_table_reader_test.o $(TEST_LIBRARY) $(LIBRARY)
$(AM_LINK)

Expand Down
8 changes: 0 additions & 8 deletions TARGETS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ cpp_library_wrapper(name="rocksdb_lib", srcs=[
"table/adaptive/adaptive_table_factory.cc",
"table/block_based/binary_search_index_reader.cc",
"table/block_based/block.cc",
"table/block_based/block_based_filter_block.cc",
"table/block_based/block_based_table_builder.cc",
"table/block_based/block_based_table_factory.cc",
"table/block_based/block_based_table_iterator.cc",
Expand Down Expand Up @@ -494,7 +493,6 @@ cpp_library_wrapper(name="rocksdb_whole_archive_lib", srcs=[
"table/adaptive/adaptive_table_factory.cc",
"table/block_based/binary_search_index_reader.cc",
"table/block_based/block.cc",
"table/block_based/block_based_filter_block.cc",
"table/block_based/block_based_table_builder.cc",
"table/block_based/block_based_table_factory.cc",
"table/block_based/block_based_table_iterator.cc",
Expand Down Expand Up @@ -4800,12 +4798,6 @@ cpp_unittest_wrapper(name="blob_garbage_meter_test",
extra_compiler_flags=[])


cpp_unittest_wrapper(name="block_based_filter_block_test",
srcs=["table/block_based/block_based_filter_block_test.cc"],
deps=[":rocksdb_test_lib"],
extra_compiler_flags=[])


cpp_unittest_wrapper(name="block_based_table_reader_test",
srcs=["table/block_based/block_based_table_reader_test.cc"],
deps=[":rocksdb_test_lib"],
Expand Down
11 changes: 4 additions & 7 deletions db/db_block_cache_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ class DBBlockCacheTest1 : public DBTestBase,
};

INSTANTIATE_TEST_CASE_P(DBBlockCacheTest1, DBBlockCacheTest1,
::testing::Values(1, 2, 3));
::testing::Values(1, 2));

TEST_P(DBBlockCacheTest1, WarmCacheWithBlocksDuringFlush) {
Options options = CurrentOptions();
Expand All @@ -686,13 +686,10 @@ TEST_P(DBBlockCacheTest1, WarmCacheWithBlocksDuringFlush) {
table_options.partition_filters = true;
table_options.index_type =
BlockBasedTableOptions::IndexType::kTwoLevelIndexSearch;
table_options.filter_policy.reset(NewBloomFilterPolicy(10, false));
table_options.filter_policy.reset(NewBloomFilterPolicy(10));
break;
case 2: // block-based filter
table_options.filter_policy.reset(NewBloomFilterPolicy(10, true));
break;
case 3: // full filter
table_options.filter_policy.reset(NewBloomFilterPolicy(10, false));
case 2: // full filter
table_options.filter_policy.reset(NewBloomFilterPolicy(10));
break;
default:
assert(false);
Expand Down
101 changes: 32 additions & 69 deletions db/db_bloom_filter_test.cc
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion db_stress_tool/db_stress_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ DECLARE_double(experimental_mempurge_threshold);
DECLARE_bool(enable_write_thread_adaptive_yield);
DECLARE_int32(reopen);
DECLARE_double(bloom_bits);
DECLARE_bool(use_block_based_filter);
DECLARE_int32(ribbon_starting_level);
DECLARE_bool(partition_filters);
DECLARE_bool(optimize_filters_for_memory);
Expand Down
4 changes: 0 additions & 4 deletions db_stress_tool/db_stress_gflags.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,10 +469,6 @@ DEFINE_double(bloom_bits, 10,
"Bloom filter bits per key. "
"Negative means use default settings.");

DEFINE_bool(use_block_based_filter, false,
"use block based filter"
"instead of full filter for block based table");

DEFINE_int32(
ribbon_starting_level, 999,
"Use Bloom filter on levels below specified and Ribbon beginning on level "
Expand Down
11 changes: 1 addition & 10 deletions db_stress_tool/db_stress_test_base.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,7 @@ std::shared_ptr<const FilterPolicy> CreateFilterPolicy() {
return BlockBasedTableOptions().filter_policy;
}
const FilterPolicy* new_policy;
if (FLAGS_use_block_based_filter) {
if (FLAGS_ribbon_starting_level < 999) {
fprintf(
stderr,
"Cannot combine use_block_based_filter and ribbon_starting_level\n");
exit(1);
} else {
new_policy = NewBloomFilterPolicy(FLAGS_bloom_bits, true);
}
} else if (FLAGS_ribbon_starting_level >= 999) {
if (FLAGS_ribbon_starting_level >= 999) {
// Use Bloom API
new_policy = NewBloomFilterPolicy(FLAGS_bloom_bits, false);
} else {
Expand Down
2 changes: 1 addition & 1 deletion include/rocksdb/cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ enum class CacheEntryRole {
kFilterBlock,
// Block-based table metadata block for partitioned filter
kFilterMetaBlock,
// Block-based table deprecated filter block (old "block-based" filter)
// OBSOLETE / DEPRECATED: old/removed block-based filter
kDeprecatedFilterBlock,
// Block-based table index block
kIndexBlock,
Expand Down
12 changes: 1 addition & 11 deletions options/options_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,21 +999,11 @@ TEST_F(OptionsTest, GetBlockBasedTableOptionsFromString) {
EXPECT_EQ(bfp->GetMillibitsPerKey(), 4000);
EXPECT_EQ(bfp->GetWholeBitsPerKey(), 4);

// Back door way of enabling deprecated block-based Bloom
ASSERT_OK(GetBlockBasedTableOptionsFromString(
config_options, table_opt,
"filter_policy=rocksdb.internal.DeprecatedBlockBasedBloomFilter:4",
&new_opt));
auto builtin =
dynamic_cast<const BuiltinFilterPolicy*>(new_opt.filter_policy.get());
EXPECT_EQ(builtin->GetId(),
"rocksdb.internal.DeprecatedBlockBasedBloomFilter:4");

// Test configuring using other internal names
ASSERT_OK(GetBlockBasedTableOptionsFromString(
config_options, table_opt,
"filter_policy=rocksdb.internal.LegacyBloomFilter:3", &new_opt));
builtin =
auto builtin =
dynamic_cast<const BuiltinFilterPolicy*>(new_opt.filter_policy.get());
EXPECT_EQ(builtin->GetId(), "rocksdb.internal.LegacyBloomFilter:3");

Expand Down
2 changes: 0 additions & 2 deletions src.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ LIB_SOURCES = \
table/adaptive/adaptive_table_factory.cc \
table/block_based/binary_search_index_reader.cc \
table/block_based/block.cc \
table/block_based/block_based_filter_block.cc \
table/block_based/block_based_table_builder.cc \
table/block_based/block_based_table_factory.cc \
table/block_based/block_based_table_iterator.cc \
Expand Down Expand Up @@ -525,7 +524,6 @@ TEST_MAIN_SOURCES = \
options/customizable_test.cc \
options/options_settable_test.cc \
options/options_test.cc \
table/block_based/block_based_filter_block_test.cc \
table/block_based/block_based_table_reader_test.cc \
table/block_based/block_test.cc \
table/block_based/data_block_hash_index_test.cc \
Expand Down
Loading