From bec4264813bd1c949be67cdba69d72143b0b25d9 Mon Sep 17 00:00:00 2001 From: anand76 Date: Wed, 21 Dec 2022 22:42:19 -0800 Subject: [PATCH] Avoid mixing sync and async prefetch (#11050) Summary: Reading uncompression dict block always uses sync reads, while data blocks may use async reads and prefetching. This causes problems in FilePrefetchBuffer. So avoid mixing the two by reading the uncompression dict straight from the file. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11050 Test Plan: Crash test Reviewed By: akankshamahajan15 Differential Revision: D42194682 Pulled By: anand1976 fbshipit-source-id: aaa8b396fdfe966b157e210f5ef8501c45b7b69e --- HISTORY.md | 1 + table/block_based/block_based_table_reader_impl.h | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 45cf403ad..ff55ebd43 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -14,6 +14,7 @@ * Fix L0 file misorder corruption caused by ingesting files of overlapping seqnos with memtable entries' through introducing `epoch_number`. Before the fix, `force_consistency_checks=true` may catch the corruption before it's exposed to readers, in which case writes returning `Status::Corruption` would be expected. Also replace the previous incomplete fix (#5958) to the same corruption with this new and more complete fix. * Fixed a bug in LockWAL() leading to re-locking mutex (#11020). * Fixed a heap use after free bug in async scan prefetching when the scan thread and another thread try to read and load the same seek block into cache. +* Fixed a heap use after free in async scan prefetching if dictionary compression is enabled, in which case sync read of the compression dictionary gets mixed with async prefetching ### New Features * When an SstPartitionerFactory is configured, CompactRange() now automatically selects for compaction any files overlapping a partition boundary that is in the compaction range, even if no actual entries are in the requested compaction range. With this feature, manual compaction can be used to (re-)establish SST partition points when SstPartitioner changes, without a full compaction. diff --git a/table/block_based/block_based_table_reader_impl.h b/table/block_based/block_based_table_reader_impl.h index 1f6f5f223..0c52973a5 100644 --- a/table/block_based/block_based_table_reader_impl.h +++ b/table/block_based/block_based_table_reader_impl.h @@ -39,9 +39,13 @@ TBlockIter* BlockBasedTable::NewDataBlockIterator( if (rep_->uncompression_dict_reader && block_type == BlockType::kData) { CachableEntry uncompression_dict; const bool no_io = (ro.read_tier == kBlockCacheTier); + // For async scans, don't use the prefetch buffer since an async prefetch + // might already be under way and this would invalidate it. Also, the + // uncompression dict is typically at the end of the file and would + // most likely break the sequentiality of the access pattern. s = rep_->uncompression_dict_reader->GetOrReadUncompressionDictionary( - prefetch_buffer, no_io, ro.verify_checksums, get_context, - lookup_context, &uncompression_dict); + ro.async_io ? nullptr : prefetch_buffer, no_io, ro.verify_checksums, + get_context, lookup_context, &uncompression_dict); if (!s.ok()) { iter->Invalidate(s); return iter;