You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "gripleaf (via GitHub)" <gi...@apache.org> on 2026/10/13 07:32:01 UTC
[PR] perf(scan): prune cached manifest partitions before decoding [paimon-cpp]
gripleaf opened a new pull request, #430:
URL: https://github.com/apache/paimon-cpp/pull/430
### Purpose
Linked issue: none.
Reduce repeated file-metadata decoding on exact snapshot manifest-cache hits when a scan selects only a small subset of a bucket's partitions.
**Scenario.** A partitioned, bucketed Paimon table serves repeated point reads. An application batches 50 requested cells, grouping cells for the same key. Each underlying lookup uses a key equality predicate and a fixed date-partition filter, allowing the planner to select one bucket. The bucket-scoped manifest cache, however, contains live files from many date partitions. Reusing that cache currently decodes the full file metadata before discarding entries outside the requested partition.
For example, one sampled bucket contains 604 live entries across 84 partitions, but only six entries belong to the queried partition:
```text
Before: deserialize 604 full entries -> apply partition filter -> keep 6
After: inspect 604 partition fields -> apply partition filter -> fully decode 6
```
The change selects the exact retained snapshot and evaluates its partition filter before constructing `ManifestEntry` / `DataFileMeta` objects and decoding file statistics. It still traverses the serialized payload; it does not eliminate all cache-read work. Scans selecting most partitions are not expected to gain the same benefit.
The shared cache retains the complete bucket contents. The filtered selection is request-local and is never written back as the cache value. Existing miss/rebuild behavior, snapshot retention, and subsequent scan filters are preserved.
**Component measurements (not end-to-end query latency).** A standalone prototype harness compared full deserialization followed by partition filtering against selective decoding, using the same serialized cache payloads from four real buckets. Each bucket had 84 partitions; each comparison selected one partition. Each path ran 200 iterations per pair, with five pairs and alternating execution order, on the same host. The table reports mean elapsed time per iteration across those pairs. Both paths returned exactly equal selected entries.
| Sample | Cached entries | Selected entries | Before (ms) | After (ms) | Time reduction |
| --- | ---: | ---: | ---: | ---: | ---: |
| A | 604 | 6 | 4.566 | 0.839 | 81.6% |
| B | 616 | 6 | 4.663 | 0.849 | 81.8% |
| C | 566 | 5 | 4.282 | 0.774 | 81.9% |
| D | 604 | 5 | 4.593 | 0.827 | 82.0% |
These measurements motivated the upstream port; they are not timings of a deployed upstream build. The port was validated by the tests below. Full cold/warm end-to-end performance and long-running concurrent behavior have not yet been benchmarked for this upstream build.
### Tests
Built the community checkout and passed **51 tests from six suites**:
```sh
cmake --build build --target paimon-core-test -j 8
./build/debug/paimon-core-test --gtest_filter='*FileStoreScanTest.*:AppendBucketPruningTest.*:ManifestEntrySerializerTest.*'
```
New regression coverage:
- `FileStoreScanTest.SelectCachedSnapshotWithoutPollutingOtherPartitions`: alternating partition filters, unfiltered reads, exact-snapshot misses, reduced retention, empty snapshots, and filter error propagation; checks that the complete serialized value remains reusable.
- `FileStoreScanTest.RejectInvalidSnapshotSelectionHeader`: empty input, invalid magic, negative counts, and a truncated header.
- `AppendOnlyFileStoreScanTest.SnapshotCachePreservesOtherPartitions`: creates a local partitioned table, populates the cache through a filtered scan, then queries another partition, an empty result, the original partition, and all partitions. Compares filtered plans with cache-disabled scans and checks the new counters.
Existing scan, bucket-pruning, snapshot cache rebuild, and serialization tests also passed. `pre-commit run --files` passed for all eight changed files; `git diff --check` passed. The repository-wide test suite was not run.
### API and Format
Adds two metric-name constants to `ScanMetrics`:
- `snapshotCacheCandidateEntries`: entries inspected in the exact retained snapshot on the selective cache-hit path.
- `snapshotCacheDecodedEntries`: entries fully decoded after partition filtering on that path.
These are per-plan counters, reset to zero when the selective path is not used. Existing lazy-scan candidate counts retain their meaning. No existing method signatures, serialized cache format, table storage format, or protocols change. The new selection helper is internal.
### Documentation
Updated `manifest_entry_cache.rst` with the selective-decoding flow, an example, shared-cache correctness guarantees, and the new metrics. Updated `metrics.rst` with the metric definitions.
### Generative AI tooling
Generated-by: OpenAI Codex (codex-cli 0.162.1)
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(scan): prune manifest partitions before metadata decoding [paimon-cpp]
Posted by "wangyong9999 (via GitHub)" <gi...@apache.org>.
wangyong9999 commented on code in PR #430:
URL: https://github.com/apache/paimon-cpp/pull/430#discussion_r4259290296
##########
src/paimon/core/operation/file_store_scan.cpp:
##########
@@ -396,44 +388,84 @@ Status FileStoreScan::ReadManifestEntries(const std::vector<ManifestFileMeta>& m
// bucket counts, with the current count and schema in the cache key. Exact hits
// can be returned directly; cache misses rebuild the target snapshot bucket from the target
// snapshot's data manifests.
-Status FileStoreScan::ReadManifestEntriesWithCache(
+Result<uint64_t> FileStoreScan::ReadManifestEntriesWithCache(
const Snapshot& snapshot, const std::vector<ManifestFileMeta>& all_manifest_metas,
int32_t bucket, std::vector<ManifestEntry>* manifest_entries, bool* cache_hit) const {
Duration cache_load_duration;
- PAIMON_ASSIGN_OR_RAISE(SnapshotLiveManifestEntries cached_entries,
- LoadSnapshotLiveManifestEntries(bucket));
+ const int32_t max_snapshots = core_options_.GetScanManifestEntryCacheMaxSnapshots();
+ SnapshotLiveManifestEntries cached_entries(max_snapshots);
+ auto row_filter = CreateManifestEntryRowFilter();
+ bool partition_filtered = false;
+ uint64_t candidate_entries = 0;
+ *cache_hit = false;
+ Result<std::shared_ptr<CacheValue>> value = core_options_.GetCache()->Get(
+ CreateSnapshotLiveManifestEntriesCacheKey(bucket),
+ [](const std::shared_ptr<CacheKey>&) -> Result<std::shared_ptr<CacheValue>> {
+ return std::shared_ptr<CacheValue>();
+ });
+ if (value.ok() && value.value()) {
+ PAIMON_ASSIGN_OR_RAISE(
+ std::optional<SnapshotLiveManifestEntries::Selection> selected,
Review Comment:
This turns a bad snapshot-cache value into a sticky scan failure. Before, `LoadSnapshotLiveManifestEntries` treated a value that failed to deserialize (bad magic, unsupported entry version, truncated rows) as a miss and rebuilt the bucket, which also overwrote the bad value. Now `ReadSnapshot` returns that error straight to `CreatePlan`, and since the miss path never runs, the same value is read again on every later scan of this bucket until it happens to be evicted.
Suggest keeping only predicate errors fatal: if `ReadSnapshot` fails while parsing the value itself, fall through to the rebuild path as before.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(scan): prune manifest partitions before metadata decoding [paimon-cpp]
Posted by "gripleaf (via GitHub)" <gi...@apache.org>.
gripleaf commented on code in PR #430:
URL: https://github.com/apache/paimon-cpp/pull/430#discussion_r4306100997
##########
src/paimon/core/operation/file_store_scan.cpp:
##########
@@ -396,44 +388,81 @@ Status FileStoreScan::ReadManifestEntries(const std::vector<ManifestFileMeta>& m
// bucket counts, with the current count and schema in the cache key. Exact hits
// can be returned directly; cache misses rebuild the target snapshot bucket from the target
// snapshot's data manifests.
-Status FileStoreScan::ReadManifestEntriesWithCache(
+Result<uint64_t> FileStoreScan::ReadManifestEntriesWithCache(
const Snapshot& snapshot, const std::vector<ManifestFileMeta>& all_manifest_metas,
int32_t bucket, std::vector<ManifestEntry>* manifest_entries, bool* cache_hit) const {
Duration cache_load_duration;
- PAIMON_ASSIGN_OR_RAISE(SnapshotLiveManifestEntries cached_entries,
- LoadSnapshotLiveManifestEntries(bucket));
+ const int32_t max_snapshots = core_options_.GetScanManifestEntryCacheMaxSnapshots();
+ SnapshotLiveManifestEntries cached_entries(max_snapshots);
+ bool predicate_failed = false;
+ auto row_filter = CreateManifestEntryRowFilter(&predicate_failed);
+ bool partition_filtered = false;
+ uint64_t candidate_entries = 0;
+ *cache_hit = false;
+ Result<std::shared_ptr<CacheValue>> value = core_options_.GetCache()->Get(
+ CreateSnapshotLiveManifestEntriesCacheKey(bucket),
+ [](const std::shared_ptr<CacheKey>&) -> Result<std::shared_ptr<CacheValue>> {
+ return std::shared_ptr<CacheValue>();
+ });
Review Comment:
Thanks for pointing this out. In our serving system, bucket is the unit of distributed computation. The original cache design followed that model, so we scoped the cached results by snapshot and bucket and overlooked the partition dimension—sorry for that oversight.
We plan to introduce partition-aware grouping and indexing in a follow-up change, aligning the cache organization with Java’s (partition, bucket) hierarchy as closely as practical. I’ll open a follow-up issue to track this work, while keeping this PR focused on reducing metadata decoding with the current layout.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(scan): prune manifest partitions before metadata decoding [paimon-cpp]
Posted by "wangyong9999 (via GitHub)" <gi...@apache.org>.
wangyong9999 commented on code in PR #430:
URL: https://github.com/apache/paimon-cpp/pull/430#discussion_r4259307284
##########
src/paimon/core/operation/file_store_scan.cpp:
##########
@@ -396,44 +388,84 @@ Status FileStoreScan::ReadManifestEntries(const std::vector<ManifestFileMeta>& m
// bucket counts, with the current count and schema in the cache key. Exact hits
// can be returned directly; cache misses rebuild the target snapshot bucket from the target
// snapshot's data manifests.
-Status FileStoreScan::ReadManifestEntriesWithCache(
+Result<uint64_t> FileStoreScan::ReadManifestEntriesWithCache(
const Snapshot& snapshot, const std::vector<ManifestFileMeta>& all_manifest_metas,
int32_t bucket, std::vector<ManifestEntry>* manifest_entries, bool* cache_hit) const {
Duration cache_load_duration;
- PAIMON_ASSIGN_OR_RAISE(SnapshotLiveManifestEntries cached_entries,
- LoadSnapshotLiveManifestEntries(bucket));
+ const int32_t max_snapshots = core_options_.GetScanManifestEntryCacheMaxSnapshots();
+ SnapshotLiveManifestEntries cached_entries(max_snapshots);
+ auto row_filter = CreateManifestEntryRowFilter();
+ bool partition_filtered = false;
+ uint64_t candidate_entries = 0;
+ *cache_hit = false;
+ Result<std::shared_ptr<CacheValue>> value = core_options_.GetCache()->Get(
+ CreateSnapshotLiveManifestEntriesCacheKey(bucket),
+ [](const std::shared_ptr<CacheKey>&) -> Result<std::shared_ptr<CacheValue>> {
+ return std::shared_ptr<CacheValue>();
+ });
+ if (value.ok() && value.value()) {
+ PAIMON_ASSIGN_OR_RAISE(
+ std::optional<SnapshotLiveManifestEntries::Selection> selected,
+ SnapshotLiveManifestEntries::ReadSnapshot(value.value()->GetSegment(), snapshot.Id(),
+ max_snapshots, row_filter, pool_));
+ if (selected) {
+ *cache_hit = true;
+ partition_filtered = static_cast<bool>(row_filter);
+ candidate_entries = selected->candidate_entries;
+ *manifest_entries = std::move(selected->entries);
+ } else {
+ // Reuse the same cache value to retain complete historical snapshots on a miss.
+ // Query-filtered entries must never be written back into the cache.
+ Result<SnapshotLiveManifestEntries> deserialized =
+ SnapshotLiveManifestEntries::Deserialize(value.value()->GetSegment(), max_snapshots,
+ pool_);
+ if (deserialized.ok()) {
+ cached_entries = std::move(deserialized.value());
+ auto cached = cached_entries.LatestBeforeOrEqual(snapshot.Id());
+ if (cached && cached->snapshot_id == snapshot.Id()) {
Review Comment:
`ReadSnapshot` has already returned no retained snapshot here. `Deserialize` reads the same bytes with the same retention limit, so this second exact-hit check cannot succeed (an empty snapshot is still a hit in `ReadSnapshot`). Please keep deserialization for preserving historical snapshots, but remove this duplicate hit branch.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(scan): prune manifest partitions before metadata decoding [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 commented on code in PR #430:
URL: https://github.com/apache/paimon-cpp/pull/430#discussion_r4305571990
##########
src/paimon/core/operation/file_store_scan.cpp:
##########
@@ -396,44 +388,81 @@ Status FileStoreScan::ReadManifestEntries(const std::vector<ManifestFileMeta>& m
// bucket counts, with the current count and schema in the cache key. Exact hits
// can be returned directly; cache misses rebuild the target snapshot bucket from the target
// snapshot's data manifests.
-Status FileStoreScan::ReadManifestEntriesWithCache(
+Result<uint64_t> FileStoreScan::ReadManifestEntriesWithCache(
const Snapshot& snapshot, const std::vector<ManifestFileMeta>& all_manifest_metas,
int32_t bucket, std::vector<ManifestEntry>* manifest_entries, bool* cache_hit) const {
Duration cache_load_duration;
- PAIMON_ASSIGN_OR_RAISE(SnapshotLiveManifestEntries cached_entries,
- LoadSnapshotLiveManifestEntries(bucket));
+ const int32_t max_snapshots = core_options_.GetScanManifestEntryCacheMaxSnapshots();
+ SnapshotLiveManifestEntries cached_entries(max_snapshots);
+ bool predicate_failed = false;
+ auto row_filter = CreateManifestEntryRowFilter(&predicate_failed);
+ bool partition_filtered = false;
+ uint64_t candidate_entries = 0;
+ *cache_hit = false;
+ Result<std::shared_ptr<CacheValue>> value = core_options_.GetCache()->Get(
+ CreateSnapshotLiveManifestEntriesCacheKey(bucket),
+ [](const std::shared_ptr<CacheKey>&) -> Result<std::shared_ptr<CacheValue>> {
+ return std::shared_ptr<CacheValue>();
+ });
Review Comment:
Thanks a lot for your contribution—this PR looks high-quality and well-tested.
I have one question. I looked at the Java code: the cache key is the manifest file path, and internally it segments and builds indexes by (partition, bucket, totalBucket).
One thing seems a bit odd: the current cache is built per (snapshot-bucket), and during queries we filter by partition. But the data is organized as (snapshot-partition-bucket). The current cache key drops the partition level; the value contains all partition for a bucket. What’s the rationale for this design?
This isn’t blocking the current PR. I’d also like to know whether the cache will be restructured in the future to match Java’s (partition, bucket)-level hierarchy.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(scan): prune manifest partitions before metadata decoding [paimon-cpp]
Posted by "wangyong9999 (via GitHub)" <gi...@apache.org>.
wangyong9999 commented on code in PR #430:
URL: https://github.com/apache/paimon-cpp/pull/430#discussion_r4259506321
##########
src/paimon/core/operation/file_store_scan.cpp:
##########
@@ -396,44 +388,84 @@ Status FileStoreScan::ReadManifestEntries(const std::vector<ManifestFileMeta>& m
// bucket counts, with the current count and schema in the cache key. Exact hits
// can be returned directly; cache misses rebuild the target snapshot bucket from the target
// snapshot's data manifests.
-Status FileStoreScan::ReadManifestEntriesWithCache(
+Result<uint64_t> FileStoreScan::ReadManifestEntriesWithCache(
const Snapshot& snapshot, const std::vector<ManifestFileMeta>& all_manifest_metas,
int32_t bucket, std::vector<ManifestEntry>* manifest_entries, bool* cache_hit) const {
Duration cache_load_duration;
- PAIMON_ASSIGN_OR_RAISE(SnapshotLiveManifestEntries cached_entries,
- LoadSnapshotLiveManifestEntries(bucket));
+ const int32_t max_snapshots = core_options_.GetScanManifestEntryCacheMaxSnapshots();
+ SnapshotLiveManifestEntries cached_entries(max_snapshots);
+ auto row_filter = CreateManifestEntryRowFilter();
+ bool partition_filtered = false;
+ uint64_t candidate_entries = 0;
+ *cache_hit = false;
+ Result<std::shared_ptr<CacheValue>> value = core_options_.GetCache()->Get(
+ CreateSnapshotLiveManifestEntriesCacheKey(bucket),
+ [](const std::shared_ptr<CacheKey>&) -> Result<std::shared_ptr<CacheValue>> {
+ return std::shared_ptr<CacheValue>();
+ });
+ if (value.ok() && value.value()) {
+ PAIMON_ASSIGN_OR_RAISE(
+ std::optional<SnapshotLiveManifestEntries::Selection> selected,
+ SnapshotLiveManifestEntries::ReadSnapshot(value.value()->GetSegment(), snapshot.Id(),
+ max_snapshots, row_filter, pool_));
+ if (selected) {
+ *cache_hit = true;
+ partition_filtered = static_cast<bool>(row_filter);
+ candidate_entries = selected->candidate_entries;
+ *manifest_entries = std::move(selected->entries);
+ } else {
+ // Reuse the same cache value to retain complete historical snapshots on a miss.
+ // Query-filtered entries must never be written back into the cache.
+ Result<SnapshotLiveManifestEntries> deserialized =
+ SnapshotLiveManifestEntries::Deserialize(value.value()->GetSegment(), max_snapshots,
+ pool_);
+ if (deserialized.ok()) {
+ cached_entries = std::move(deserialized.value());
+ auto cached = cached_entries.LatestBeforeOrEqual(snapshot.Id());
+ if (cached && cached->snapshot_id == snapshot.Id()) {
Review Comment:
Confirmed in 8151e02c: the redundant hit branch is removed while deserialization still preserves historical snapshots. Resolved.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(scan): prune manifest partitions before metadata decoding [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 merged PR #430:
URL: https://github.com/apache/paimon-cpp/pull/430
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org