wangyong9999 commented on code in PR #277:
URL: https://github.com/apache/paimon-cpp/pull/277#discussion_r3921990677
##########
include/paimon/table/source/plan.h:
##########
@@ -34,5 +35,13 @@ class PAIMON_EXPORT Plan {
virtual const std::vector<std::shared_ptr<Split>>& Splits() const = 0;
/// Snapshot id of this plan, return `std::nullopt` if the table is empty.
virtual std::optional<int64_t> SnapshotId() const = 0;
+ /// Immutable snapshot metadata used to build this plan.
+ ///
+ /// The returned view can be injected into a later batch scan to reuse the
already resolved and
+ /// parsed snapshot. Only non-streaming, non-real-time latest-snapshot
plans publish a reusable
+ /// view; other plans return `nullptr`.
+ virtual std::shared_ptr<const SnapshotReadView> GetSnapshotReadView()
const {
+ return nullptr;
+ }
Review Comment:
Thanks — the case 2 vs. case 3 comparison was the right boundary to test. I
ran a controlled point-lookup planning benchmark at the current PR head
(`e13ba033`) and the incremental hot-cache benefit is modest.
```mermaid
pie title Case 2 P50 share with warmed local metadata
"Planning unchanged by the View — 1.965 ms" : 93.51
"Eliminated by SnapshotReadView — 0.136 ms" : 6.49
```
### Setup
- Release build: GCC 8.3, `-O3 -DNDEBUG`.
- ORC primary-key fixture, full-PK equality predicate, bucket 0, one
resulting split.
- The same warmed 64 MiB LRU cache and
`scan.manifest-entry-cache.max-snapshots=8` in all three modes.
- 200 warm-up iterations, then 3,000 measured iterations per mode and
scenario, interleaved round-robin and pinned to one CPU.
- The filesystem wrapper counted logical metadata method calls by path
category. All three modes produced the same split count; after warm-up,
manifest/index I/O was zero.
### Warm local metadata
| Mode | P50 | P99 | Single-thread planning ops/s | Snapshot calls | Schema
calls | List calls |
|---|---:|---:|---:|---:|---:|---:|
| 1. normal latest scan | 2.119 ms | 2.234 ms | 470.9 | 4 | 3 | 1 |
| 2. `scan.snapshot-id` + `SetTableSchema()` | 2.101 ms | 2.211 ms | 474.5 |
7 | 0 | 0 |
| 3. `SnapshotReadView` | 1.965 ms | 2.078 ms | 507.5 | 0 | 0 | 0 |
The incremental case 2 → case 3 result is:
- P50: **-6.49%** (136 μs)
- P99: **-5.99%** (133 μs)
- single-thread planning throughput: **+6.95%**
This is planning throughput, not end-to-end serving QPS. Since the View does
not change data-file access, adding the common read/decode work would make its
relative end-to-end share smaller in this hot-metadata boundary.
### Controlled latency sensitivity
I also injected delay before each counted logical metadata operation to
identify whether the benefit is local parsing/object construction or avoided
filesystem round trips:
| Injected delay / metadata call | Case 2 P50 / P99 | Case 3 P50 / P99 | P50
/ P99 reduction |
|---|---:|---:|---:|
| 0 | 2.101 / 2.211 ms | 1.965 / 2.078 ms | 6.49% / 5.99% |
| 100 μs | 3.152 / 3.241 ms | 1.961 / 2.026 ms | 37.78% / 37.48% |
| 1 ms | 9.455 / 9.563 ms | 1.962 / 2.023 ms | 79.24% / 78.85% |
The 100 μs and 1 ms rows are synthetic sensitivity tests, **not** measured
HDFS latency. They show that the proposal becomes valuable when those seven
logical metadata calls carry real RTT, but they do not establish that
production boundary.
My conclusion is that the measured hot-cache incremental gain does not
justify the current composite public API/ABI by itself. I will stop pursuing
this version and close the PR. If this is revisited, the prerequisite should be
a real remote-filesystem end-to-end measurement, and I would first explore a
narrower immutable snapshot-metadata cache rather than exposing
`SnapshotReadView` as a public cross-scan object.
--
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]
