perf(scan): lazily decode manifest bucket entries - #212
Conversation
lxy-9602
left a comment
lxy-9602
left a comment
There was a problem hiding this comment.
Thank you for the contribution! I reviewed the code, and the overall design looks fine to me. While I’d like to better understand the practical benefit of this PR in production. For example, in a scenario where we scan 1k manifests and query a specific bucket, roughly how much scan latency reduction can we expect?
| for (int64_t i = 0; i < batch->length(); i++) { | ||
| ColumnarRow row(batch->fields(), pool_, i); | ||
| PAIMON_RETURN_NOT_OK(ManifestEntrySerializer::ValidateVersion(row.GetInt(0))); | ||
| if (row.GetInt(3) != bucket) { |
There was a problem hiding this comment.
Please add a bucketGetter in ManifestEntrySerializer, similar to the Java implementation, to avoid using the raw index 3 directly.
| reader_builder_->Build(file_input_stream)); | ||
| auto reader = std::make_unique<ManifestMetaReader>(std::move(batch_reader), | ||
| serializer_->GetDataType(), pool_); | ||
| ScopeGuard close_reader([&reader]() { reader->Close(); }); |
There was a problem hiding this comment.
We probably don’t need ScopeGuard here, since cleanup is already handled by ManifestMetaReader’s destructor via DoClose.
This optimization mainly improves cold-start performance. In our case, we have around 16k buckets, but only need the manifest for a single bucket. The latency improvement is roughly comparable to the difference between P99 Phase 1 and Current Phase 2 in the figure, which represents about a 60% reduction in latency.
|
|
Got it! It does look like manifest deserialization takes a significant amount of time. Please address the review comments so the PR can move forward and be merged. |
a2fa79b to
59e2dd3
Compare
|
Please link issue id and fix the wrong format of PR desc (lines after |
done |
4 participants
Purpose
Optimize snapshot live manifest cache rebuilding for bucket-filtered scans.
Previously, rebuilding a bucket-scoped cache entry deserialized every manifest entry before filtering by bucket. This change reads the serialization version and bucket first, and performs full
ManifestEntrydeserialization only for entries belonging to the target bucket.The optimization is enabled by default and can be disabled with
scan.manifest-entry.lazy-decode.enabled.This change also:
Tests
Added or updated unit tests covering:
scan.manifest-entry.lazy-decode.enabled.All 8 selected unit tests passed.
API and Format
This change adds the following public configuration API:
The corresponding option key is
scan.manifest-entry.lazy-decode.enabled, and its default value is true.
This change does not modify the manifest storage format, serialization version, or external protocol.
Documentation
The new configuration option and affected internal APIs include English inline documentation.
No standalone user documentation is added because this option controls an internal scan optimization and does not change scan results.
Generative AI tooling
Generated-by: OpenAI Codex (GPT-5)