feat: add local disk cache#600
Conversation
leaves12138
left a comment
There was a problem hiding this comment.
Re-reviewed the current head and reran the focused cache tests (33/33 passed). I still found two blocking cases that are not covered by the current tests.
| .unwrap_or(DEFAULT_FILE_SIZE_CAPACITY) | ||
| .clamp(1, DEFAULT_FILE_SIZE_CAPACITY); | ||
| Ok(Self { | ||
| disk: DiskCache::new(config.dir.join(CACHE_DIRECTORY_NAME), config.max_size)?, |
There was a problem hiding this comment.
Each catalog creates an independent DiskCache and CacheState here, even when multiple catalogs in the same process share the same configured local-cache.dir. If two catalogs are constructed before either writes, both scan an empty directory; with a one-block local-cache.max-size, each can subsequently retain one block, so total encoded disk usage becomes twice the configured limit. Their LRU views also become stale relative to each other. The documentation explicitly allows catalogs to share the base directory, so namespace isolation alone does not preserve the size/LRU contract. Could we share the disk cache state per canonical cache root within the process (and handle conflicting limits), or physically isolate capacity accounting per catalog? A two-cache-instance test with one shared directory would catch this.
| discovered: &mut Vec<(std::time::SystemTime, PathBuf, BlockKey, u64)>, | ||
| ) { | ||
| let path = entry.path(); | ||
| let encoded = match std::fs::read(&path) { |
There was a problem hiding this comment.
DiskCache::new reaches this path synchronously, and startup reads every cached block in full; decode_block_any then validates the CRC over the complete payload. With the documented 20 GiB cache example, catalog construction can synchronously read roughly 20 GiB and block the async caller before the catalog is usable. Could startup recover the key/size from bounded metadata and validate the payload lazily on the first hit, or otherwise move the full scan off the blocking construction path?
leaves12138
left a comment
There was a problem hiding this comment.
Re-reviewed commit 9acccf0. The shared per-root cache state now enforces one LRU/size limit across catalog instances, and restart recovery runs on a blocking worker with header-only scanning and lazy payload CRC validation. I also verified the namespace/invalidation changes and ran formatting, the focused cache/FileIO/REST tests, and the full paimon library test suite (1869 passed, 1 ignored). The previous concerns are addressed.
What changed
local-cache.enabled,local-cache.dir,local-cache.max-size,local-cache.block-size, andlocal-cache.whitelistoptionsFileIOWhy
Repeated reads of immutable Paimon metadata and index ranges currently go back to the underlying storage every time. This adds an opt-in local disk cache to reduce remote requests and repeated read latency while keeping storage as the source of truth.
User impact
The feature is disabled by default. When enabled, metadata and global-index files are cached by default. Runtime cache failures fail open to the original storage, while mutable markers and temporary files bypass caching.
Validation
cargo test -p paimon --lib— 1861 passed, 1 ignoredcargo clippy -p paimon --all-targets -- -D warningscargo fmt --all -- --checkgit diff --check