diff --git a/file/file_prefetch_buffer.cc b/file/file_prefetch_buffer.cc index 50b5edcb213..aa84bb7c928 100644 --- a/file/file_prefetch_buffer.cc +++ b/file/file_prefetch_buffer.cc @@ -92,10 +92,9 @@ void FilePrefetchBuffer::PrepareBufferForRead( Status FilePrefetchBuffer::Read(BufferInfo* buf, const IOOptions& opts, RandomAccessFileReader* reader, - const uint64_t original_length, uint64_t read_len, uint64_t aligned_useful_len, - uint64_t start_offset, bool use_fs_buffer, - bool for_compaction) { + uint64_t optional_read_size, + uint64_t start_offset, bool use_fs_buffer) { Slice result; Status s; char* to_buf = nullptr; @@ -104,11 +103,9 @@ Status FilePrefetchBuffer::Read(BufferInfo* buf, const IOOptions& opts, read_len, result); } else { to_buf = buf->buffer_.BufferStart() + aligned_useful_len; - bool issue_flexible_read = for_compaction && !reader->use_direct_io() && - original_length < read_len; - if (issue_flexible_read) { - s = FlexibleMultiRead(reader, opts, start_offset + aligned_useful_len, - original_length, read_len, to_buf, result); + if (0 < optional_read_size) { + s = FlexibleRead(reader, opts, start_offset + aligned_useful_len, + read_len, optional_read_size, to_buf, result); } else { s = reader->Read(opts, start_offset + aligned_useful_len, read_len, &result, to_buf, /*aligned_buf=*/nullptr); @@ -205,9 +202,13 @@ Status FilePrefetchBuffer::Prefetch(const IOOptions& opts, Status s; if (read_len > 0) { - s = Read(buf, opts, reader, /*original_length=*/n, read_len, - aligned_useful_len, rounddown_offset, use_fs_buffer, - /*for_compaction=*/false); + // Currently FilePrefetchBuffer::Prefetch is used in + // BlockBasedTable::PrefetchTail. Our optimization for FlexibleRead is meant + // for when we want to start from the beginning of the file in compaction + // and read the whole file sequentially. It is probably not worth setting + // optimal_read_size > 0 in this case. + s = Read(buf, opts, reader, read_len, aligned_useful_len, + /*optional_read_size=*/0, rounddown_offset, use_fs_buffer); } if (usage_ == FilePrefetchBufferUsage::kTableOpenPrefetchTail && s.ok()) { @@ -745,8 +746,12 @@ Status FilePrefetchBuffer::PrefetchInternal(const IOOptions& opts, } if (read_len1 > 0) { - s = Read(buf, opts, reader, original_length, read_len1, aligned_useful_len1, - start_offset1, use_fs_buffer, for_compaction); + size_t optional_read_length = for_compaction && !reader->use_direct_io() && + original_length < read_len1 + ? read_len1 - original_length + : 0; + s = Read(buf, opts, reader, read_len1, aligned_useful_len1, + optional_read_length, start_offset1, use_fs_buffer); if (!s.ok()) { AbortAllIOs(); FreeAllBuffers(); diff --git a/file/file_prefetch_buffer.h b/file/file_prefetch_buffer.h index 41023cf2618..a8d0f73b6b7 100644 --- a/file/file_prefetch_buffer.h +++ b/file/file_prefetch_buffer.h @@ -437,9 +437,9 @@ class FilePrefetchBuffer { bool for_compaction, bool& copy_to_third_buffer); Status Read(BufferInfo* buf, const IOOptions& opts, - RandomAccessFileReader* reader, const uint64_t original_length, - uint64_t read_len, uint64_t aligned_useful_len, - uint64_t start_offset, bool use_fs_buffer, bool for_compaction); + RandomAccessFileReader* reader, uint64_t read_len, + uint64_t aligned_useful_len, uint64_t optional_read_size, + uint64_t start_offset, bool use_fs_buffer); Status ReadAsync(BufferInfo* buf, const IOOptions& opts, RandomAccessFileReader* reader, uint64_t read_len, @@ -541,16 +541,18 @@ class FilePrefetchBuffer { return s; } - IOStatus FlexibleMultiRead(RandomAccessFileReader* reader, - const IOOptions& opts, uint64_t offset, - size_t original_length, size_t read_length, - char* scratch, Slice& result) { + // FlexibleRead enables the result size to be in the range of + // [read_length - optional_read_size, read_length] + IOStatus FlexibleRead(RandomAccessFileReader* reader, const IOOptions& opts, + uint64_t offset, size_t read_length, + size_t optional_read_size, char* scratch, + Slice& result) { FSReadRequest read_req; read_req.offset = offset; read_req.len = read_length; read_req.scratch = scratch; - assert(original_length <= read_length); - read_req.optional_read_size = read_length - original_length; + assert(optional_read_size <= read_length); + read_req.optional_read_size = optional_read_size; IOStatus s = reader->MultiRead(opts, &read_req, 1, nullptr); if (!s.ok()) { return s; diff --git a/include/rocksdb/file_system.h b/include/rocksdb/file_system.h index ce285b78ff3..57c1667bbfc 100644 --- a/include/rocksdb/file_system.h +++ b/include/rocksdb/file_system.h @@ -818,7 +818,7 @@ struct FSReadRequest { // returned when the end of file has been reached or an error has occurred. // // When optional_read_size > 0, if the end of the file is not reached, the - // returned data can have size in the range [len - optional_read_size, len] + // returned data's size can be in the range [len - optional_read_size, len] // // Note that optional_read_size should never exceed len // @@ -883,10 +883,11 @@ struct FSReadRequest { // WARNING 2: Since fs_scratch is a unique pointer, FSReadRequest's copy // constructor is implicitly disabled. This turns out to be very useful // because we want users to be explicit when setting offset, len, and - // optional_read_size. If you end up wanting to delete this field, be very - // careful and consider explicitly deleting the copy constructor, since the - // lack of a copy constructor is likely acting as a good protective measure - // against bugs + // optional_read_size. Consider the possibility where optional_read_size ends + // up exceeding the request length because it was copied over by mistake. If + // you end up wanting to delete this field, be very careful and consider + // explicitly deleting the copy constructor, since the lack of a copy + // constructor is likely acting as a good protective measure against bugs FSAllocationPtr fs_scratch; };