Make some FilterPolicy deprecations more clear (#9403)
Summary: The old block-based filter has been deprecated for years, but this makes that more clear by marking the functions specific to it and logging a warning when the feature is used. It is deprecated because of performance. In that old design, you have to binary search through the full SST index before a bloom filter query, which is much more expensive than a bloom query itself. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9403 Test Plan: Used db_bench with and without -use_block_based_filter, running at the same time TEST_TMPDIR=/dev/shm/rocksdb ./db_bench -benchmarks=fillrandom,readrandom -num=10000000 -duration=20 -disable_wal=1 -write_buffer_size=10000000 -bloom_bits=16 -compaction_style=2 -fifo_compaction_max_table_files_size_mb=10000 -fifo_compaction_allow_compaction=0 No significant difference in construction time but 3x slower readrandom with -use_block_based_filter: readrandom : 100.517 micros/op 9948 ops/sec; 1.1 MB/s vs. readrandom : 33.368 micros/op 29968 ops/sec; 3.3 MB/s Also saw deprecation message (just once) in LOG only with -use_block_based_filter Reviewed By: ajkr Differential Revision: D33673202 Pulled By: pdillinger fbshipit-source-id: 99f6f0eff619408d9e5f7ef546954ed0be6c7a5b
This commit is contained in:
parent
875bfd75a0
commit
ffe1e4b820
@ -177,6 +177,9 @@ class FilterPolicy {
|
|||||||
// passed to methods of this type.
|
// passed to methods of this type.
|
||||||
virtual const char* Name() const = 0;
|
virtual const char* Name() const = 0;
|
||||||
|
|
||||||
|
// DEPRECATED: This function is part of the deprecated block-based
|
||||||
|
// filter, which will be removed in a future release.
|
||||||
|
//
|
||||||
// keys[0,n-1] contains a list of keys (potentially with duplicates)
|
// keys[0,n-1] contains a list of keys (potentially with duplicates)
|
||||||
// that are ordered according to the user supplied comparator.
|
// that are ordered according to the user supplied comparator.
|
||||||
// Append a filter that summarizes keys[0,n-1] to *dst.
|
// Append a filter that summarizes keys[0,n-1] to *dst.
|
||||||
@ -186,6 +189,9 @@ class FilterPolicy {
|
|||||||
virtual void CreateFilter(const Slice* keys, int n,
|
virtual void CreateFilter(const Slice* keys, int n,
|
||||||
std::string* dst) const = 0;
|
std::string* dst) const = 0;
|
||||||
|
|
||||||
|
// DEPRECATED: This function is part of the deprecated block-based
|
||||||
|
// filter, which will be removed in a future release.
|
||||||
|
//
|
||||||
// "filter" contains the data appended by a preceding call to
|
// "filter" contains the data appended by a preceding call to
|
||||||
// CreateFilter() on this class. This method must return true if
|
// CreateFilter() on this class. This method must return true if
|
||||||
// the key was in the list of keys passed to CreateFilter().
|
// the key was in the list of keys passed to CreateFilter().
|
||||||
@ -198,6 +204,7 @@ class FilterPolicy {
|
|||||||
// NOTE: This function is only called by GetBuilderWithContext() below for
|
// NOTE: This function is only called by GetBuilderWithContext() below for
|
||||||
// custom FilterPolicy implementations. Thus, it is not necessary to
|
// custom FilterPolicy implementations. Thus, it is not necessary to
|
||||||
// override this function if overriding GetBuilderWithContext().
|
// override this function if overriding GetBuilderWithContext().
|
||||||
|
// DEPRECATED: This function will be removed in a future release.
|
||||||
virtual FilterBitsBuilder* GetFilterBitsBuilder() const { return nullptr; }
|
virtual FilterBitsBuilder* GetFilterBitsBuilder() const { return nullptr; }
|
||||||
|
|
||||||
// A newer variant of GetFilterBitsBuilder that allows a FilterPolicy
|
// A newer variant of GetFilterBitsBuilder that allows a FilterPolicy
|
||||||
@ -282,7 +289,7 @@ extern const FilterPolicy* NewBloomFilterPolicy(
|
|||||||
extern const FilterPolicy* NewRibbonFilterPolicy(
|
extern const FilterPolicy* NewRibbonFilterPolicy(
|
||||||
double bloom_equivalent_bits_per_key, int bloom_before_level = 0);
|
double bloom_equivalent_bits_per_key, int bloom_before_level = 0);
|
||||||
|
|
||||||
// Old name and old default behavior
|
// Old name and old default behavior (DEPRECATED)
|
||||||
inline const FilterPolicy* NewExperimentalRibbonFilterPolicy(
|
inline const FilterPolicy* NewExperimentalRibbonFilterPolicy(
|
||||||
double bloom_equivalent_bits_per_key) {
|
double bloom_equivalent_bits_per_key) {
|
||||||
return NewRibbonFilterPolicy(bloom_equivalent_bits_per_key, -1);
|
return NewRibbonFilterPolicy(bloom_equivalent_bits_per_key, -1);
|
||||||
|
@ -1257,6 +1257,13 @@ FilterBitsBuilder* BloomFilterPolicy::GetBuilderWithContext(
|
|||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
case kDeprecatedBlock:
|
case kDeprecatedBlock:
|
||||||
|
if (context.info_log && !warned_.load(std::memory_order_relaxed)) {
|
||||||
|
warned_ = true;
|
||||||
|
ROCKS_LOG_WARN(context.info_log,
|
||||||
|
"Using deprecated block-based Bloom filter is "
|
||||||
|
"inefficient (%d bits per key).",
|
||||||
|
whole_bits_per_key_);
|
||||||
|
}
|
||||||
return nullptr;
|
return nullptr;
|
||||||
case kFastLocalBloom:
|
case kFastLocalBloom:
|
||||||
return new FastLocalBloomBitsBuilder(
|
return new FastLocalBloomBitsBuilder(
|
||||||
|
Loading…
Reference in New Issue
Block a user