Skip to content

Commit 439e170

Browse files
cbi42facebook-github-bot
authored andcommitted
Fix MultiScan Prepare() to support dictionary compression (facebook#13896)
Summary: I saw failure when added some asserts near https://github.com/facebook/rocksdb/blob/b9957c991cae44959f96888369caf1b145398132/table/block_based/block_based_table_iterator.cc#L1201-L1205 in stress test. The decompression failed with error message like "Corruption: Failed zlib inflate: -3". This PR fixes the issue to use the right decompressor for dictionary compression. Pull Request resolved: facebook#13896 Test Plan: updated unit test that checks no I/O is done after Prepare(), this would fail before this change. Reviewed By: anand1976 Differential Revision: D80821500 Pulled By: cbi42 fbshipit-source-id: a4322c0da99a2d10e9787d0ec168668567c0c19a
1 parent 239b06c commit 439e170

3 files changed

Lines changed: 51 additions & 3 deletions

File tree

table/block_based/block_based_table_iterator.cc

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,6 +1175,34 @@ void BlockBasedTableIterator::Prepare(const MultiScanArgs* multiscan_opts) {
11751175
}
11761176
}
11771177

1178+
// Get compression dictionary if available - needed for dictionary-aware
1179+
// decompression
1180+
UnownedPtr<Decompressor> decompressor =
1181+
table_->get_rep()->decompressor.get();
1182+
CachableEntry<DecompressorDict> cached_dict;
1183+
if (table_->get_rep()->uncompression_dict_reader) {
1184+
s = table_->get_rep()
1185+
->uncompression_dict_reader->GetOrReadUncompressionDictionary(
1186+
/* prefetch_buffer= */ nullptr, read_options_,
1187+
/* get_context= */ nullptr, /* lookup_context= */ nullptr,
1188+
&cached_dict);
1189+
if (!s.ok()) {
1190+
#ifndef NDEBUG
1191+
fprintf(stdout, "Prepare dictionary loading failed with %s\n",
1192+
s.ToString().c_str());
1193+
#endif
1194+
// Abort: dictionary lookup failed.
1195+
return;
1196+
}
1197+
if (!cached_dict.GetValue()) {
1198+
#ifndef NDEBUG
1199+
fprintf(stdout, "Success but no dictionary read\n");
1200+
#endif
1201+
return;
1202+
}
1203+
decompressor = cached_dict.GetValue()->decompressor_.get();
1204+
}
1205+
11781206
// Init blocks and pin them in block cache.
11791207
MemoryAllocator* memory_allocator =
11801208
table_->get_rep()->table_options.block_cache->memory_allocator();
@@ -1199,10 +1227,12 @@ void BlockBasedTableIterator::Prepare(const MultiScanArgs* multiscan_opts) {
11991227
#endif
12001228
assert(pinned_data_blocks_guard[block_idx].IsEmpty());
12011229
s = table_->CreateAndPinBlockInCache<Block_kData>(
1202-
read_options_, block, table_->get_rep()->decompressor.get(),
1203-
&tmp_contents,
1230+
read_options_, block, decompressor, &tmp_contents,
12041231
&(pinned_data_blocks_guard[block_idx].As<Block_kData>()));
12051232
if (!s.ok()) {
1233+
#ifndef NDEBUG
1234+
fprintf(stdout, "Prepare failed with %s\n", s.ToString().c_str());
1235+
#endif
12061236
// Abort: failed to create and pin block in cache
12071237
return;
12081238
}

table/block_based/block_based_table_reader_test.cc

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -994,6 +994,18 @@ TEST_P(BlockBasedTableReaderTestVerifyChecksum, ChecksumMismatch) {
994994
}
995995

996996
TEST_P(BlockBasedTableReaderTest, MultiScanPrepare) {
997+
std::ostringstream param_trace;
998+
param_trace << "[MultiScanPrepare] Test params: " << "CompressionType="
999+
<< CompressionTypeToString(compression_type_)
1000+
<< ", UseDirectReads=" << (use_direct_reads_ ? "true" : "false")
1001+
<< ", UDTEnabled=" << (udt_enabled_ ? "true" : "false")
1002+
<< ", PersistUDT=" << (persist_udt_ ? "true" : "false")
1003+
<< ", CompressionParallelThreads="
1004+
<< compression_parallel_threads_
1005+
<< ", CompressionDictBytes=" << compression_dict_bytes_
1006+
<< ", SameKeyDiffTs=" << (same_key_diff_ts_ ? "true" : "false");
1007+
std::cout << param_trace.str() << std::endl;
1008+
9971009
Options options;
9981010
options.statistics = CreateDBStatistics();
9991011
ReadOptions read_opts;
@@ -1052,6 +1064,9 @@ TEST_P(BlockBasedTableReaderTest, MultiScanPrepare) {
10521064
iter->Next();
10531065
}
10541066
ASSERT_OK(iter->status());
1067+
// No I/O expected during scanning since all blocks were loaded and pinned.
1068+
ASSERT_EQ(read_count_after,
1069+
options.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT));
10551070

10561071
iter.reset(table->NewIterator(
10571072
read_opts, options_.prefix_extractor.get(), /*arena=*/nullptr,
@@ -1089,7 +1104,7 @@ TEST_P(BlockBasedTableReaderTest, MultiScanPrepare) {
10891104
read_opts, options_.prefix_extractor.get(), /*arena=*/nullptr,
10901105
/*skip_filters=*/false, TableReaderCaller::kUncategorized));
10911106
// Should do two I/Os since blocks 80-81 and 90-95 are already in block cache,
1092-
// reads from blocks 50-79 and 82-.. are co
1107+
// reads from blocks 50-79 and 82-.. are coalesced.
10931108
scan_options = MultiScanArgs(BytewiseComparator());
10941109
scan_options.insert(ExtractUserKey(kv[50 * kEntriesPerBlock].first));
10951110
read_count_before =
@@ -1106,6 +1121,8 @@ TEST_P(BlockBasedTableReaderTest, MultiScanPrepare) {
11061121
}
11071122
ASSERT_FALSE(iter->Valid());
11081123
ASSERT_OK(iter->status());
1124+
ASSERT_EQ(read_count_after,
1125+
options.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT));
11091126

11101127
// Check cases when Seek key does not match start key in ScanOptions
11111128
iter.reset(table->NewIterator(
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* Fix a bug in MultiScan that causes it to fall back to a normal scan when dictionary compression is enabled.

0 commit comments

Comments
 (0)