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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
