Skip to content

perf(scan): reuse index planning resources across queries - #267

Open
wangyong9999 wants to merge 5 commits into
apache:mainfrom
wangyong9999:perf/reuse-index-snapshot-caches
Open

perf(scan): reuse index planning resources across queries#267
wangyong9999 wants to merge 5 commits into
apache:mainfrom
wangyong9999:perf/reuse-index-snapshot-caches

Conversation

@wangyong9999

@wangyong9999 wangyong9999 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: N/A

Repeated scans in the same process rebuilt several immutable planning resources:

  • DataEvolutionBatchScan loaded the schema and rebuilt CoreOptions after TableScan had already initialized them. This also dropped the cache supplied through ScanContext.
  • snapshot live manifest entries were cached only when a bucket filter was present.
  • each BTree indexer created a new 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 CacheManager between 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-size for each distinct cache configuration. The existing eager BlockCache constructor and Close() behavior are unchanged.

On an HDFS primary-key table with 606K rows, 51 active data files, and 6.5 GB of data, a logid lookup 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

  • debug build: core_test, common_test, and sst_format_test
  • AppendOnlyFileStoreScanTest.TestSnapshotLiveManifestCache* (4 cases)
  • LruCacheTest.TestForSnapshotLiveManifestEntries
  • BlockCacheTest.* (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)
  • two-query native-HDFS validation: validation status=PASS, path_proof status=PASS path=pk-file-local-btree
  • git diff --check

The 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::ForSnapshotLiveManifestEntries for 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)

@wangyong9999

Copy link
Copy Markdown
Contributor Author

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;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +94 to +95
std::shared_ptr<CacheManager> cache_manager =
GetSharedCacheManager(cache_size, high_priority_pool_ratio);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +359 to +362
} else {
std::vector<ManifestEntry> unmerged_entries;
PAIMON_RETURN_NOT_OK(
ReadFileEntries(all_manifest_metas, &unmerged_entries, /*apply_scan_filter=*/false));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant