Skip to content

Commit 97ff8ed

Browse files
committed
[refactor](be) Align file cache probe results with read blocks
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The async cache-write planner queried one block-aligned range, but `BlockFileCache::probe` returned a `FileBlocksHolder` of cache hits plus an independent gap list. The planner then had to scan all hits and gaps for every logical read block even though the read-plan blocks and probe slots use the same block boundaries. This obscured the alignment invariant and introduced unnecessary nested matching logic on the query read path. Change the probe contract to return one ordered nullable `FileBlock` pointer per aligned input block. A non-null slot is asserted to have the exact corresponding range, except that the final block may end at EOF, while a null slot directly represents a cache miss. The planner now preserves its inflight-first fast path and joins probe results to plan blocks by index; materialization also reads the matching slot directly. Remove `FileBlocksHolder` and the independent gaps from `FileBlocksProbeResult`. Preserve the existing deferred cleanup semantics for EMPTY and deleting cache blocks through a shared cache-user reference release helper rather than embedding a holder in the probe result. Update focused and end-to-end tests for hit/miss slots, a short final block, retained block states, self-heal cleanup, and an aligned direct-cache prefix followed by an async-written suffix. ### Release note None ### Check List (For Author) - Test: Unit Test - 51 related ASAN BE unit tests passed with `run-be-ut.sh` and `-j100` - 2 focused probe/direct-prefix ASAN BE unit tests passed with `run-be-ut.sh` and `-j100` - `build-support/check-format.sh` passed - Behavior changed: No; this simplifies an internal probe/planning contract without changing user-visible cache semantics - Does this need documentation: No
1 parent 8cbf214 commit 97ff8ed

9 files changed

Lines changed: 212 additions & 207 deletions

be/src/io/cache/block_file_cache.cpp

Lines changed: 27 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -845,9 +845,9 @@ Status BlockFileCache::get_downloaded_blocks_if_fully_covered(const UInt128Wrapp
845845
FileBlocksProbeResult BlockFileCache::probe(const UInt128Wrapper& hash, size_t offset, size_t size,
846846
const CacheContext& context) {
847847
DORIS_CHECK(size > 0);
848-
const FileBlock::Range range(offset, offset + size - 1);
849-
FileBlocks blocks;
850-
std::vector<FileBlock::Range> gaps;
848+
DORIS_CHECK(_max_file_block_size > 0);
849+
const size_t end = offset + size;
850+
DORIS_CHECK(end > offset);
851851
std::lock_guard cache_lock(_mutex);
852852

853853
auto file_iterator = _files.find(hash);
@@ -860,46 +860,36 @@ FileBlocksProbeResult BlockFileCache::probe(const UInt128Wrapper& hash, size_t o
860860
_storage->load_blocks_directly_unlocked(this, key, cache_lock);
861861
file_iterator = _files.find(hash);
862862
}
863-
if (file_iterator == _files.end()) {
864-
gaps.emplace_back(range);
865-
return FileBlocksProbeResult(std::move(blocks), std::move(gaps));
866-
}
867863

868-
auto& file_blocks = file_iterator->second;
869-
DORIS_CHECK(!file_blocks.empty());
870-
auto block_iterator = file_blocks.lower_bound(range.left);
871-
if (block_iterator != file_blocks.begin()) {
872-
auto previous = std::prev(block_iterator);
873-
if (previous->second.file_block->range().right >= range.left) {
874-
block_iterator = previous;
864+
FileBlocksByOffset* cached_blocks = nullptr;
865+
FileBlocksByOffset::iterator cached_block;
866+
if (file_iterator != _files.end()) {
867+
DORIS_CHECK(!file_iterator->second.empty());
868+
cached_blocks = &file_iterator->second;
869+
cached_block = cached_blocks->lower_bound(offset);
870+
if (cached_block != cached_blocks->begin()) {
871+
const auto& previous_range = std::prev(cached_block)->second.file_block->range();
872+
DORIS_CHECK(previous_range.right < offset);
875873
}
876874
}
877875

878-
size_t current = range.left;
879-
for (; block_iterator != file_blocks.end(); ++block_iterator) {
880-
const auto& block = block_iterator->second.file_block;
881-
const auto& block_range = block->range();
882-
if (block_range.left > range.right) {
883-
break;
884-
}
885-
if (block_range.right < range.left) {
886-
continue;
876+
std::vector<FileBlockSPtr> result;
877+
result.reserve(size / _max_file_block_size + (size % _max_file_block_size != 0));
878+
for (size_t block_offset = offset; block_offset < end;) {
879+
const size_t block_size = std::min(_max_file_block_size, end - block_offset);
880+
const FileBlock::Range expected_range(block_offset, block_offset + block_size - 1);
881+
FileBlockSPtr file_block;
882+
if (cached_blocks != nullptr && cached_block != cached_blocks->end() &&
883+
cached_block->second.file_block->range().left <= expected_range.right) {
884+
file_block = cached_block->second.file_block;
885+
DORIS_CHECK(file_block->range().left == expected_range.left);
886+
DORIS_CHECK(file_block->range().right == expected_range.right);
887+
++cached_block;
887888
}
888-
const size_t clipped_left = std::max(block_range.left, range.left);
889-
const size_t clipped_right = std::min(block_range.right, range.right);
890-
if (current < clipped_left) {
891-
gaps.emplace_back(current, clipped_left - 1);
892-
}
893-
blocks.emplace_back(block);
894-
current = clipped_right + 1;
895-
if (current > range.right) {
896-
break;
897-
}
898-
}
899-
if (current <= range.right) {
900-
gaps.emplace_back(current, range.right);
889+
result.emplace_back(std::move(file_block));
890+
block_offset += block_size;
901891
}
902-
return FileBlocksProbeResult(std::move(blocks), std::move(gaps));
892+
return FileBlocksProbeResult(std::move(result));
903893
}
904894

905895
void BlockFileCache::touch_probe_block_if_cached(const FileBlockSPtr& block,

be/src/io/cache/block_file_cache.h

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <boost/lockfree/spsc_queue.hpp>
2727
#include <functional>
2828
#include <limits>
29+
#include <map>
2930
#include <memory>
3031
#include <mutex>
3132
#include <optional>
@@ -83,15 +84,17 @@ class LockScopedTimer {
8384
class FSFileCacheStorage;
8485

8586
struct FileBlocksProbeResult {
86-
FileBlocksProbeResult(FileBlocks blocks, std::vector<FileBlock::Range> gaps_)
87-
: holder(std::move(blocks)), gaps(std::move(gaps_)) {}
87+
explicit FileBlocksProbeResult(std::vector<FileBlockSPtr> file_blocks_)
88+
: file_blocks(std::move(file_blocks_)) {}
8889
FileBlocksProbeResult(FileBlocksProbeResult&&) noexcept = default;
8990
FileBlocksProbeResult& operator=(FileBlocksProbeResult&&) noexcept = delete;
9091
FileBlocksProbeResult(const FileBlocksProbeResult&) = delete;
9192
FileBlocksProbeResult& operator=(const FileBlocksProbeResult&) = delete;
93+
~FileBlocksProbeResult();
9294

93-
FileBlocksHolder holder;
94-
std::vector<FileBlock::Range> gaps;
95+
/// One entry per cache-block-sized input slot, in offset order. A null entry is a cache miss;
96+
/// a non-null entry has the exact slot range, except that the final slot may end at EOF.
97+
std::vector<FileBlockSPtr> file_blocks;
9598
};
9699

97100
// NeedUpdateLRUBlocks keeps FileBlockSPtr entries that require LRU updates in a
@@ -254,8 +257,10 @@ class BlockFileCache {
254257
FileBlocksHolder get_or_set(const UInt128Wrapper& hash, size_t offset, size_t size,
255258
CacheContext& context);
256259

257-
/// Return existing blocks and uncovered gaps for `[offset, offset + size)` without creating
258-
/// cache cells or touching LRU state. `context` supplies cache-type visibility rules.
260+
/// Probe the block-aligned `[offset, offset + size)` range without creating cache cells or
261+
/// touching LRU state. The result contains one ordered slot per cache block; each slot is null
262+
/// on miss or owns the exactly aligned existing block. `context` supplies cache metadata when
263+
/// lazy loading is required.
259264
FileBlocksProbeResult probe(const UInt128Wrapper& hash, size_t offset, size_t size,
260265
const CacheContext& context);
261266

be/src/io/cache/cached_remote_file_reader.h

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -170,33 +170,34 @@ class CachedRemoteFileReader final : public FileReader,
170170

171171
/// Build block-aligned source coverage for the unread suffix. It first performs one inflight
172172
/// batch lookup; a fully covered request returns without taking the BlockFileCache lock, while
173-
/// incomplete coverage triggers one read-only cache probe for the whole aligned range.
173+
/// incomplete coverage triggers one read-only whole-range probe with one result per aligned
174+
/// block.
174175
/// @param[in] remaining_offset First user byte not filled by the direct-cache path.
175176
/// @param[in] remaining_size Number of unread user bytes.
176177
/// @param[in] write_epoch Epoch captured before any lookup or remote IO.
177178
/// @param[in] io_ctx Context used to build the cache admission/probe context.
178179
/// @param[in,out] stats Lookup and probe counters updated during planning.
179-
/// @return Plan that owns any required probe holder and first-to-last remote range.
180+
/// @return Plan that owns any retained probe blocks and first-to-last remote range.
180181
AsyncReadPlan _build_async_read_plan(size_t remaining_offset, size_t remaining_size,
181182
uint64_t write_epoch, const IOContext* io_ctx,
182183
ReadStatistics& stats);
183184

184185
/// Copy one block already available from an inflight buffer or downloaded cache file. Cache
185186
/// state is revalidated before IO; a race is reported to the caller as a simple full-range
186187
/// remote fallback.
187-
/// @param[in] plan Plan owning the probe holder and user boundaries.
188-
/// @param[in] read_block Aligned block classified as INFLIGHT, CACHE, or DOWNLOADING.
188+
/// @param[in] plan Plan owning the probed blocks and user boundaries.
189+
/// @param[in] block_index Index of the aligned block and its matching probe result.
189190
/// @param[in] user_offset Original user request offset used to locate the destination slice.
190191
/// @param[out] result Destination buffer for the complete user request.
191192
/// @param[in] cache_context Context used only when a successful local read touches LRU.
192193
/// @param[in,out] stats Local-read timing counters.
193194
/// @param[in,out] materialized_bytes User bytes copied from cache or inflight memory.
194195
/// @param[in,out] need_self_heal Set when a cache file disappears during a local read.
195196
/// @return true when the block was copied; false when the caller should use remote fallback.
196-
bool _materialize_async_block(const AsyncReadPlan& plan, const AsyncReadBlock& read_block,
197-
size_t user_offset, Slice result,
198-
const CacheContext& cache_context, ReadStatistics& stats,
199-
size_t* materialized_bytes, bool* need_self_heal);
197+
bool _materialize_async_block(const AsyncReadPlan& plan, size_t block_index, size_t user_offset,
198+
Slice result, const CacheContext& cache_context,
199+
ReadStatistics& stats, size_t* materialized_bytes,
200+
bool* need_self_heal);
200201

201202
/// Copy only blocks before the first and after the last REMOTE block. When no REMOTE block
202203
/// exists, copy the entire request. DOWNLOADING blocks outside the remote span keep the

be/src/io/cache/cached_remote_file_reader_async_write.cpp

Lines changed: 74 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,7 @@ CacheWriteMode CachedRemoteFileReader::_resolve_cache_write_mode(const IOContext
122122

123123
// Build a deliberately small plan. The inflight index is checked first so a fully covered read
124124
// avoids BlockFileCache::probe and its cache mutex. Only an incomplete inflight lookup performs one
125-
// whole-range cache probe; the per-block scans favor readability because read_at normally spans
126-
// very few cache blocks.
125+
// whole-range probe whose result entries map directly to the logical plan blocks.
127126
CachedRemoteFileReader::AsyncReadPlan CachedRemoteFileReader::_build_async_read_plan(
128127
size_t remaining_offset, size_t remaining_size, uint64_t write_epoch,
129128
const IOContext* io_ctx, ReadStatistics& stats) {
@@ -140,7 +139,8 @@ CachedRemoteFileReader::AsyncReadPlan CachedRemoteFileReader::_build_async_read_
140139
block_offset += cache_block_size) {
141140
const size_t block_size = std::min(cache_block_size, size() - block_offset);
142141
DORIS_CHECK(block_size > 0);
143-
plan.blocks.emplace_back(FileBlock::Range(block_offset, block_offset + block_size - 1));
142+
const FileBlock::Range block_range(block_offset, block_offset + block_size - 1);
143+
plan.blocks.emplace_back(block_range);
144144
block_offsets.emplace_back(block_offset);
145145
}
146146
DORIS_CHECK(!plan.blocks.empty());
@@ -180,42 +180,42 @@ CachedRemoteFileReader::AsyncReadPlan CachedRemoteFileReader::_build_async_read_
180180
plan.probe_result.emplace(_cache->probe(_cache_hash, align_left, align_size, cache_context));
181181
g_cached_remote_reader_probe_total << 1;
182182
const auto& probe_result = *plan.probe_result;
183+
DORIS_CHECK(probe_result.file_blocks.size() == plan.blocks.size());
183184

184-
const auto overlaps = [](const FileBlock::Range& lhs, const FileBlock::Range& rhs) {
185-
return lhs.left <= rhs.right && rhs.left <= lhs.right;
186-
};
187185
for (size_t index = 0; index < plan.blocks.size(); ++index) {
188186
auto& read_block = plan.blocks[index];
189187
if (read_block.source == AsyncReadBlock::Source::INFLIGHT) {
190188
continue;
191189
}
192190

193-
bool has_miss = std::any_of(
194-
probe_result.gaps.begin(), probe_result.gaps.end(),
195-
[&](const FileBlock::Range& gap) { return overlaps(gap, read_block.range); });
196-
bool has_downloading = false;
197-
bool has_cache_block = false;
198-
for (const auto& block : probe_result.holder.file_blocks) {
199-
if (!overlaps(block->range(), read_block.range)) {
200-
continue;
201-
}
202-
has_cache_block = true;
203-
if (_cache->is_block_deleting(block)) {
204-
has_miss = true;
191+
const auto& file_block = probe_result.file_blocks[index];
192+
bool is_miss = file_block == nullptr;
193+
bool is_downloading = false;
194+
if (file_block != nullptr) {
195+
DORIS_CHECK(file_block->range().left == read_block.range.left);
196+
DORIS_CHECK(file_block->range().right == read_block.range.right);
197+
if (_cache->is_block_deleting(file_block)) {
198+
is_miss = true;
205199
} else {
206-
const FileBlock::State state = block->state();
207-
has_miss = has_miss || state == FileBlock::State::EMPTY ||
208-
state == FileBlock::State::SKIP_CACHE;
209-
has_downloading = has_downloading || state == FileBlock::State::DOWNLOADING;
200+
switch (file_block->state()) {
201+
case FileBlock::State::DOWNLOADED:
202+
break;
203+
case FileBlock::State::DOWNLOADING:
204+
is_downloading = true;
205+
break;
206+
case FileBlock::State::EMPTY:
207+
case FileBlock::State::SKIP_CACHE:
208+
is_miss = true;
209+
break;
210+
}
210211
}
211212
}
212-
DORIS_CHECK(has_miss || has_cache_block);
213213

214-
if (has_miss) {
214+
if (is_miss) {
215215
read_block.submit_write = true;
216216
++stats.probe_miss;
217217
g_cached_remote_reader_probe_miss << 1;
218-
} else if (has_downloading) {
218+
} else if (is_downloading) {
219219
read_block.source = AsyncReadBlock::Source::DOWNLOADING;
220220
++stats.probe_downloading_hit;
221221
g_cached_remote_reader_probe_downloading << 1;
@@ -238,10 +238,14 @@ CachedRemoteFileReader::AsyncReadPlan CachedRemoteFileReader::_build_async_read_
238238
// Copy one block available from inflight memory or cache. DOWNLOADING blocks outside the remote
239239
// span retain the existing wait semantics. Any race or read failure asks the caller to replace the
240240
// whole planned request with one remote read instead of incrementally repairing the range.
241-
bool CachedRemoteFileReader::_materialize_async_block(
242-
const AsyncReadPlan& plan, const AsyncReadBlock& read_block, size_t user_offset,
243-
Slice result, const CacheContext& cache_context, ReadStatistics& stats,
244-
size_t* materialized_bytes, bool* need_self_heal) {
241+
bool CachedRemoteFileReader::_materialize_async_block(const AsyncReadPlan& plan, size_t block_index,
242+
size_t user_offset, Slice result,
243+
const CacheContext& cache_context,
244+
ReadStatistics& stats,
245+
size_t* materialized_bytes,
246+
bool* need_self_heal) {
247+
DORIS_CHECK(block_index < plan.blocks.size());
248+
const auto& read_block = plan.blocks[block_index];
245249
DORIS_CHECK(read_block.source != AsyncReadBlock::Source::REMOTE);
246250
DORIS_CHECK(materialized_bytes != nullptr);
247251
DORIS_CHECK(need_self_heal != nullptr);
@@ -265,64 +269,54 @@ bool CachedRemoteFileReader::_materialize_async_block(
265269
}
266270

267271
DORIS_CHECK(plan.probe_result.has_value());
268-
size_t local_read_bytes = 0;
269-
std::vector<FileBlockSPtr> consumed_blocks;
270-
for (const auto& block : plan.probe_result->holder.file_blocks) {
271-
if (block->range().right < copy_left || block->range().left > copy_right) {
272-
continue;
273-
}
274-
if (_cache->is_block_deleting(block)) {
275-
return false;
276-
}
272+
DORIS_CHECK(block_index < plan.probe_result->file_blocks.size());
273+
const auto& file_block = plan.probe_result->file_blocks[block_index];
274+
DORIS_CHECK(file_block != nullptr);
275+
DORIS_CHECK(file_block->range().left == read_block.range.left);
276+
DORIS_CHECK(file_block->range().right == read_block.range.right);
277+
if (_cache->is_block_deleting(file_block)) {
278+
return false;
279+
}
277280

278-
FileBlock::State state = block->state();
279-
if (state == FileBlock::State::DOWNLOADING) {
280-
DORIS_CHECK(read_block.source == AsyncReadBlock::Source::DOWNLOADING);
281-
{
282-
SCOPED_RAW_TIMER(&stats.remote_wait_timer);
283-
state = block->wait();
284-
}
285-
if (state != FileBlock::State::DOWNLOADED) {
286-
++stats.block_wait_timeout;
287-
g_cached_remote_reader_block_wait_timeout << 1;
288-
return false;
289-
}
290-
++stats.block_wait_success;
291-
g_cached_remote_reader_block_wait << 1;
281+
FileBlock::State state = file_block->state();
282+
if (state == FileBlock::State::DOWNLOADING) {
283+
DORIS_CHECK(read_block.source == AsyncReadBlock::Source::DOWNLOADING);
284+
{
285+
SCOPED_RAW_TIMER(&stats.remote_wait_timer);
286+
state = file_block->wait();
292287
}
293288
if (state != FileBlock::State::DOWNLOADED) {
289+
++stats.block_wait_timeout;
290+
g_cached_remote_reader_block_wait_timeout << 1;
294291
return false;
295292
}
293+
++stats.block_wait_success;
294+
g_cached_remote_reader_block_wait << 1;
295+
}
296+
if (state != FileBlock::State::DOWNLOADED) {
297+
return false;
298+
}
296299

297-
const size_t read_left = std::max(block->range().left, copy_left);
298-
const size_t read_right = std::min(block->range().right, copy_right);
299-
const size_t read_size = read_right - read_left + 1;
300-
Status status;
301-
{
302-
SCOPED_RAW_TIMER(&stats.local_read_timer);
303-
status = block->read(Slice(result.data + (read_left - user_offset), read_size),
304-
read_left - block->range().left);
305-
}
306-
if (!status.ok()) {
307-
if (status.is<ErrorCode::NOT_FOUND>()) {
308-
*need_self_heal = true;
309-
g_read_cache_self_heal_on_not_found << 1;
310-
}
311-
LOG_EVERY_N(WARNING, 100)
312-
<< "Read probed file cache block failed, falling back to remote. path="
313-
<< path().native() << ", hash=" << _cache_hash.to_string()
314-
<< ", offset=" << block->offset() << ", status=" << status;
315-
return false;
300+
Status status;
301+
{
302+
SCOPED_RAW_TIMER(&stats.local_read_timer);
303+
status = file_block->read(Slice(result.data + (copy_left - user_offset), copy_size),
304+
copy_left - file_block->range().left);
305+
}
306+
if (!status.ok()) {
307+
if (status.is<ErrorCode::NOT_FOUND>()) {
308+
*need_self_heal = true;
309+
g_read_cache_self_heal_on_not_found << 1;
316310
}
317-
local_read_bytes += read_size;
318-
consumed_blocks.emplace_back(block);
311+
LOG_EVERY_N(WARNING, 100)
312+
<< "Read probed file cache block failed, falling back to remote. path="
313+
<< path().native() << ", hash=" << _cache_hash.to_string()
314+
<< ", offset=" << file_block->offset() << ", status=" << status;
315+
return false;
319316
}
320-
DORIS_CHECK(local_read_bytes == copy_size);
321317

322-
for (const auto& block : consumed_blocks) {
323-
_cache->touch_probe_block_if_cached(block, cache_context);
324-
}
325-
*materialized_bytes += local_read_bytes;
318+
_cache->touch_probe_block_if_cached(file_block, cache_context);
319+
*materialized_bytes += copy_size;
326320
return true;
327321
}
328322

@@ -339,9 +333,8 @@ bool CachedRemoteFileReader::_materialize_async_cached_sides(
339333
size_t materialized_bytes = 0;
340334
const auto materialize_range = [&](size_t begin, size_t end) {
341335
for (size_t index = begin; index < end; ++index) {
342-
if (!_materialize_async_block(plan, plan.blocks[index], user_offset, result,
343-
cache_context, stats, &materialized_bytes,
344-
need_self_heal)) {
336+
if (!_materialize_async_block(plan, index, user_offset, result, cache_context, stats,
337+
&materialized_bytes, need_self_heal)) {
345338
return false;
346339
}
347340
}

0 commit comments

Comments
 (0)