perf(scan): reuse index planning resources across queries - #267
perf(scan): reuse index planning resources across queries#267wangyong9999 wants to merge 5 commits into
Conversation
|
The failing integration test shared the process-wide BTree cache with an earlier case, so its file-open count can be zero on a warm cache. I updated the assertion while retaining the indexed-split, bounded-range, selected-row, and read-result checks. The new fork workflows are waiting for maintainer approval. |
| double high_priority_pool_ratio) { | ||
| using CacheConfig = std::pair<int64_t, double>; | ||
| static std::mutex mutex; | ||
| static std::map<CacheConfig, SharedCacheManager> cache_managers; |
There was a problem hiding this comment.
cache_managers owns a full LRU for every distinct table-level (cache_size, ratio) forever. Each cache is bounded, but the process total is not; reading tables with different settings keeps all previous budgets resident. Please use one process-wide budget, or bound and evict this registry.
| std::shared_ptr<CacheManager> cache_manager = | ||
| GetSharedCacheManager(cache_size, high_priority_pool_ratio); |
There was a problem hiding this comment.
This manager is shared across all indexers with the same size settings, but page keys contain only file_path/offset/length. Two custom FileSystem or reader backends can expose the same logical path with different bytes, so the second scan can reuse the first backend's pages without opening its own file and return wrong matches. Include a stable backend namespace in the key, or scope the manager by backend.
| core_options_.GetScanManifestEntryCacheMaxSnapshots() > 0 && | ||
| core_options_.GetCache() != nullptr && !table_path_.empty() && | ||
| !row_range_index_.has_value() && bucket_filter_.has_value(); | ||
| !row_range_index_.has_value(); |
There was a problem hiding this comment.
The whole-table key is only (table_path, branch), and a hit is accepted by numeric snapshot ID. After a table is dropped and recreated at the same path, snapshot IDs restart while the caller-owned cache can still hold the old entry, so an ordinary scan can return stale splits. Include a table or snapshot generation identity (for example the manifest-list name) in the cached entry/key, or invalidate it on recreation.
| } else { | ||
| std::vector<ManifestEntry> unmerged_entries; | ||
| PAIMON_RETURN_NOT_OK( | ||
| ReadFileEntries(all_manifest_metas, &unmerged_entries, /*apply_scan_filter=*/false)); |
There was a problem hiding this comment.
On this no-bucket miss we materialize every manifest entry, then serialize up to max_snapshots full table states. If that value exceeds the cache capacity, Put fails silently and every later scan of the same snapshot repeats the full read, merge, copy, and serialization. Only use this path when the value can be retained; otherwise fall back to filtered_manifest_file_metas, or store snapshots separately so older states can be evicted.
Purpose
Linked issue: N/A
Repeated scans in the same process rebuilt several immutable planning resources:
DataEvolutionBatchScanloaded the schema and rebuiltCoreOptionsafterTableScanhad already initialized them. This also dropped the cache supplied throughScanContext.CacheManager; closing a reader invalidated its pages, and the next reader opened the index file before checking the cache.This change reuses the
TableSchema,CoreOptions, filesystem, executor, memory pool, and cache already owned by the scan when planning a DataEvolution global-index query.It adds a whole-table snapshot live manifest cache entry, separate from bucket-specific entries. The cache stores live entries before query filtering and applies the current predicate after a cache hit.
It shares a bounded BTree
CacheManagerbetween indexers with the same cache configuration, retains immutable pages after reader destruction, and opens the input stream only on a cache miss. Retained pages use the process default memory pool so they do not outlive a caller-owned pool.The manifest cache remains disabled for row-range scans. Cache read, deserialization, serialization, or write failures fall back to rebuilding the manifest entries. Cache-manager creation is serialized; the existing LRU remains thread-safe and bounded by
btree-index.cache-sizefor each distinct cache configuration. The existing eagerBlockCacheconstructor andClose()behavior are unchanged.On an HDFS primary-key table with 606K rows, 51 active data files, and 6.5 GB of data, a
logidlookup returning two rows was executed twice in one process. The cold plan/read/total time was 851/163/1015 ms. The second plan/read/total time was 115/99/214 ms. BTree evaluation decreased from 621 ms to 0.55 ms. Both runs selected one indexed split, one data file, and a two-row range, and matched the result of an index-disabled scan.Tests
core_test,common_test, andsst_format_testAppendOnlyFileStoreScanTest.TestSnapshotLiveManifestCache*(4 cases)LruCacheTest.TestForSnapshotLiveManifestEntriesBlockCacheTest.*(7 cases)LazyFilteredBTreeReaderTest.*(33 cases)*BTree*(144 passed; 7 existing compatibility-data cases skipped by their fixture guard)*GlobalIndexTest.TestDataEvolutionBatchScan*(32 cases covering partition/index combinations, external paths, bitmap, and range bitmap)validation status=PASS,path_proof status=PASS path=pk-file-local-btreegit diff --checkThe local environment does not provide CMake or pre-commit executables, so the CMake and pre-commit checks are left to CI.
API and Format
Adds an overload of
CacheKey::ForSnapshotLiveManifestEntriesfor a whole-table cache key. This is an additive API change. There is no object-layout, storage-format, or protocol change.Documentation
No new user-facing feature or option.
Generative AI tooling
Generated-by: Codex (GPT-5)