github-actions[bot] commented on code in PR #63376:
URL: https://github.com/apache/doris/pull/63376#discussion_r3407478084
##########
be/src/common/config.cpp:
##########
@@ -1196,6 +1196,8 @@ DEFINE_Bool(enable_file_cache, "false");
// Both will use the directory "memory" on the disk instead of the real RAM.
DEFINE_String(file_cache_path, "[{\"path\":\"${DORIS_HOME}/file_cache\"}]");
DEFINE_Int64(file_cache_each_block_size, "1048576"); // 1MB
+DEFINE_Bool(enable_external_file_meta_disk_cache, "true");
+DEFINE_mInt64(external_file_meta_disk_cache_max_entry_bytes, "67108864"); //
64MB
Review Comment:
This compiled default is `true`, but the newly added `conf/be.conf` example
documents `# enable_external_file_meta_disk_cache = false`, and the PR
description frames the behavior as applying when the flag is enabled. Because
`ExecEnv::init_file_cache_factory()` now initializes `FileCacheFactory`
whenever this flag is true, a default BE with `enable_file_cache=false` will
still create/use `file_cache_path` for persisted external footers. Please align
the compiled default with the documented opt-in behavior, or update the config
sample/release note/tests to make the default-on disk usage explicit.
##########
be/src/io/fs/file_meta_cache.cpp:
##########
@@ -17,27 +17,380 @@
#include "io/fs/file_meta_cache.h"
+#include <crc32c/crc32c.h>
+#include <gen_cpp/Types_types.h>
+
+#include <algorithm>
+#include <cstring>
+#include <utility>
+
+#include "common/config.h"
+#include "common/logging.h"
+#include "io/cache/block_file_cache.h"
+#include "io/cache/block_file_cache_factory.h"
+#include "io/cache/file_cache_common.h"
+#include "util/coding.h"
+#include "util/defer_op.h"
+#include "util/slice.h"
+#include "util/stopwatch.hpp"
+
namespace doris {
+namespace {
-std::string FileMetaCache::get_key(const std::string file_name, int64_t
modification_time,
- int64_t file_size) {
- std::string meta_cache_key;
- meta_cache_key.resize(file_name.size() + sizeof(int64_t));
+constexpr std::string_view FILE_META_CACHE_DISK_MAGIC = "DFMC";
+constexpr uint8_t FILE_META_CACHE_DISK_VERSION = 1;
+constexpr size_t FILE_META_CACHE_DISK_HEADER_SIZE = 4 + 1 + 1 + 2 + 8 + 8 + 8
+ 4;
- memcpy(meta_cache_key.data(), file_name.data(), file_name.size());
- if (modification_time != 0) {
- memcpy(meta_cache_key.data() + file_name.size(), &modification_time,
sizeof(int64_t));
- } else {
- memcpy(meta_cache_key.data() + file_name.size(), &file_size,
sizeof(int64_t));
+std::string_view format_name(FileMetaCacheFormat format) {
+ switch (format) {
+ case FileMetaCacheFormat::PARQUET:
+ return "parquet";
+ case FileMetaCacheFormat::ORC:
+ return "orc";
+ }
+ DCHECK(false) << "unknown file meta cache format";
+ return "unknown";
+}
+
+struct FileMetaCacheDiskHeader {
+ FileMetaCacheFormat format;
+ int64_t modification_time = 0;
+ int64_t file_size = 0;
+ uint64_t payload_size = 0;
+ uint32_t checksum = 0;
+};
+
+Status parse_disk_cache_header(std::string_view header,
FileMetaCacheDiskHeader* parsed) {
+ DCHECK(header.size() == FILE_META_CACHE_DISK_HEADER_SIZE);
+ if (std::memcmp(header.data(), FILE_META_CACHE_DISK_MAGIC.data(),
+ FILE_META_CACHE_DISK_MAGIC.size()) != 0) {
+ return Status::NotFound("file meta disk cache magic mismatch");
+ }
+
+ const auto* ptr =
+ reinterpret_cast<const uint8_t*>(header.data() +
FILE_META_CACHE_DISK_MAGIC.size());
+ const uint8_t version = *ptr++;
+ if (version != FILE_META_CACHE_DISK_VERSION) {
+ return Status::NotFound("file meta disk cache version mismatch");
+ }
+
+ parsed->format = static_cast<FileMetaCacheFormat>(*ptr++);
+ ptr += 2;
+ parsed->file_size = static_cast<int64_t>(decode_fixed64_le(ptr));
+ ptr += sizeof(uint64_t);
+ parsed->modification_time = static_cast<int64_t>(decode_fixed64_le(ptr));
+ ptr += sizeof(uint64_t);
+ parsed->payload_size = decode_fixed64_le(ptr);
+ ptr += sizeof(uint64_t);
+ parsed->checksum = decode_fixed32_le(ptr);
+ return Status::OK();
+}
+
+std::string build_disk_cache_value(FileMetaCacheFormat format, int64_t
modification_time,
+ int64_t file_size, std::string_view
payload) {
+ std::string value;
+ value.reserve(FILE_META_CACHE_DISK_HEADER_SIZE + payload.size());
+ value.append(FILE_META_CACHE_DISK_MAGIC.data(),
FILE_META_CACHE_DISK_MAGIC.size());
+ value.push_back(static_cast<char>(FILE_META_CACHE_DISK_VERSION));
+ value.push_back(static_cast<char>(format));
+ value.push_back(0);
+ value.push_back(0);
+ put_fixed64_le(&value, static_cast<uint64_t>(file_size));
+ put_fixed64_le(&value, static_cast<uint64_t>(modification_time));
+ put_fixed64_le(&value, payload.size());
+ put_fixed32_le(&value, crc32c::Crc32c(payload.data(), payload.size()));
+ value.append(payload.data(), payload.size());
+ return value;
+}
+
+io::CacheContext build_meta_cache_context() {
+ io::CacheContext context;
+ context.cache_type = io::FileCacheType::INDEX;
+ context.query_id = TUniqueId();
+ context.expiration_time = 0;
+ context.is_cold_data = false;
+ context.is_warmup = false;
+ return context;
+}
+
+void update_profile_counter(int64_t* counter, int64_t value = 1) {
+ if (counter != nullptr) {
+ *counter += value;
}
+}
+
+Status read_cached_file_cache(io::BlockFileCache* cache, const
io::UInt128Wrapper& hash,
+ size_t offset, Slice buffer) {
+ if (buffer.size == 0) {
+ return Status::OK();
+ }
+
+ auto blocks = cache->get_blocks_by_key(hash);
+ Defer reset_owned_by_cached_reader {[&] {
+ for (auto& [_, block] : blocks) {
+ block->_owned_by_cached_reader = false;
+ }
+ }};
+ if (blocks.empty()) {
+ return Status::NotFound("file cache block not found, hash={}",
hash.to_string());
+ }
+
+ const size_t right = offset + buffer.size - 1;
+ size_t current_pos = offset;
+ size_t written_size = 0;
+ auto it = blocks.upper_bound(offset);
+ if (it != blocks.begin()) {
+ --it;
+ }
+ for (; it != blocks.end() && current_pos <= right; ++it) {
+ const auto& block = it->second;
+ const auto& range = block->range();
+ if (range.right < current_pos) {
+ continue;
+ }
+ if (range.left > current_pos) {
+ return Status::NotFound("file cache block range has holes,
hash={}", hash.to_string());
+ }
+
+ const size_t read_size = std::min(range.right, right) - current_pos +
1;
+ const size_t read_offset = current_pos - range.left;
+ RETURN_IF_ERROR(block->read(Slice(buffer.data + written_size,
read_size), read_offset));
+ written_size += read_size;
Review Comment:
`read_cached_file_cache()` is now the hot path for L2 footer hits, but it
only reads blocks returned by `get_blocks_by_key()` and never touches them in
`BlockFileCache`. Unlike `get_or_set()`/`CachedRemoteFileReader` direct reads,
this bypasses `use_cell()`/`add_need_update_lru_block()`, so repeated
Parquet/ORC disk-footer hits do not update LRU or `atime`. Under INDEX queue
pressure those hot footer entries can still be evicted as the oldest blocks,
which defeats the persistent cache after it starts working. Please touch each
successfully read block, for example via `add_need_update_lru_block()` or a
proper read-if-cached API that updates LRU state, before returning a persisted
hit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
