Skip to content

Commit 27d3bc1

Browse files
committed
Use a different approach to make sure BlockBasedTableReader can use hash index on older files
Summary: A recent commit facebook/rocksdb@e37dd21 makes sure hash index can be used when reading existing files. This patch uses another way to achieve the approach: (1) Currently, always writing kBinarySearch to files, despite of BlockBasedTableOptions.IndexType setting. (2) When reading a file, read out the field, and make sure it is kBinarySearch, while always use index type by users. The reason for doing it is, to reserve kHashSearch property on disk to future. If now we write out binary index for both of kHashSearch and kBinarySearch. We have to use a new flag in the future for hash index on disk, otherwise compatibility would break. Also, we want the real index type and type shown in properties block to be consistent. Test Plan: make all check Reviewers: haobo, kailiu Reviewed By: kailiu CC: igor, ljin, yhchiang, xjin, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D18009
1 parent 35c968f commit 27d3bc1

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

table/block_based_table_builder.cc

+4-4
Original file line numberDiff line numberDiff line change
@@ -159,9 +159,6 @@ IndexBuilder* CreateIndexBuilder(IndexType type, const Comparator* comparator) {
159159
case BlockBasedTableOptions::kBinarySearch: {
160160
return new ShortenedIndexBuilder(comparator);
161161
}
162-
case BlockBasedTableOptions::kHashSearch: {
163-
return new ShortenedIndexBuilder(comparator);
164-
}
165162
default: {
166163
assert(!"Do not recognize the index type ");
167164
return nullptr;
@@ -324,13 +321,16 @@ struct BlockBasedTableBuilder::Rep {
324321
}
325322
};
326323

324+
// TODO(sdong): Currently only write out binary search index. In
325+
// BlockBasedTableReader, Hash index will be built using binary search index.
327326
BlockBasedTableBuilder::BlockBasedTableBuilder(
328327
const Options& options, const BlockBasedTableOptions& table_options,
329328
const InternalKeyComparator& internal_comparator, WritableFile* file,
330329
CompressionType compression_type)
331330
: rep_(new Rep(options, internal_comparator, file,
332331
table_options.flush_block_policy_factory.get(),
333-
compression_type, table_options.index_type)) {
332+
compression_type,
333+
BlockBasedTableOptions::IndexType::kBinarySearch)) {
334334
if (rep_->filter_block != nullptr) {
335335
rep_->filter_block->StartBlock(0);
336336
}

table/block_based_table_reader.cc

+17-2
Original file line numberDiff line numberDiff line change
@@ -1030,14 +1030,29 @@ bool BlockBasedTable::TEST_KeyInCache(const ReadOptions& options,
10301030
Status BlockBasedTable::CreateIndexReader(IndexReader** index_reader) {
10311031
// Some old version of block-based tables don't have index type present in
10321032
// table properties. If that's the case we can safely use the kBinarySearch.
1033-
auto index_type = rep_->index_type;
1033+
auto index_type_on_file = BlockBasedTableOptions::kBinarySearch;
1034+
if (rep_->table_properties) {
1035+
auto& props = rep_->table_properties->user_collected_properties;
1036+
auto pos = props.find(BlockBasedTablePropertyNames::kIndexType);
1037+
if (pos != props.end()) {
1038+
index_type_on_file = static_cast<BlockBasedTableOptions::IndexType>(
1039+
DecodeFixed32(pos->second.c_str()));
1040+
}
1041+
}
1042+
1043+
// TODO(sdong): Currently binary index is the only index type we support in
1044+
// files. Hash index is built on top of binary index too.
1045+
if (index_type_on_file != BlockBasedTableOptions::kBinarySearch) {
1046+
return Status::NotSupported("File Contains not supported index type: ",
1047+
std::to_string(index_type_on_file));
1048+
}
10341049

10351050
auto file = rep_->file.get();
10361051
const auto& index_handle = rep_->index_handle;
10371052
auto env = rep_->options.env;
10381053
auto comparator = &rep_->internal_comparator;
10391054

1040-
switch (index_type) {
1055+
switch (rep_->index_type) {
10411056
case BlockBasedTableOptions::kBinarySearch: {
10421057
return BinarySearchIndexReader::Create(file, index_handle, env,
10431058
comparator, index_reader);

0 commit comments

Comments
 (0)