lucasfang opened a new pull request, #338:
URL: https://github.com/apache/paimon-cpp/pull/338
### Purpose
Linked issue: close #xxx
Scan planning paid several avoidable object-store round trips: it
re-resolved file lengths the metadata already records, probed existence before
listing directories that already list as empty when missing, asked Jindo for a
directory's status twice, and read the base and delta manifest lists one after
the other. This removes all four.
Changes:
- `src/paimon/core/utils/objects_file.h`: `Read` / `ReadIfFileExist` /
`ReadArrowBatches` / `ReadFileSegment` take an optional known `file_size`
(default `std::nullopt`), and a new `OpenForRead` helper opens with
`Open(FileStatus(path, size))` when the length is known and falls back to
`Open(path)` when it is not.
- `src/paimon/core/manifest/manifest_file.{h,cpp}`: `ReadBucketEntries`
forwards the optional size.
- `src/paimon/core/manifest/manifest_list.h`:
`ReadBase/Delta/ChangelogManifests` pass the manifest-list sizes recorded on
the snapshot.
- `src/paimon/core/operation/file_store_scan.cpp` and
`src/paimon/core/snapshot_file_scan.cpp`: pass each
`ManifestFileMeta::FileSize()` to the manifest reads; in `ScanMode::ALL`, read
the base and delta manifest lists concurrently through the existing `executor_`
(`Via` + `CollectAll`), preserving the base-then-delta order the serial
`ReadDataManifests` produced.
- `src/paimon/core/utils/file_utils.cpp`: drop the `Exists()` probe before
`ListDir` in `ListVersionedFileStatus`.
- `src/paimon/fs/jindo/jindo_file_system.cpp`: collapse `ListDir`'s
`Exists()` + `GetFileStatus()` into a single `GetFileStatus`, mapping not-found
to an empty listing and propagating any other error.
The size fields stay optional, so metadata written before they existed keeps
reading through the `Open(path)` fallback — this is an optimization, not a new
requirement on the metadata.
### Tests
- New UTs: `ManifestFileTest.TestReadPassesKnownSizeToOpen`,
`ManifestListTest.TestReadDataManifestsOpensWithSizeFromSnapshot` and
`ManifestListTest.TestReadDataManifestsWithoutSizesStillReads`,
`SnapshotFileScanTest.TestListFilesOpensManifestsWithKnownLength`,
`FileUtilsTest.TestListVersionedFilesListsWithoutProbingExistence` and
`FileUtilsTest.TestListVersionedFilesMissingDirListsWithoutProbingExistence`.
They assert the known length reaches `Open(FileStatus)`, that unknown sizes
fall back to `Open(path)`, and that a listing no longer probes existence —
including for a missing directory.
- Actual results: `cmake --build build-debug-bench --target paimon-core-test
-j 96` and `--target paimon-fs-test -j 96` both built clean. `paimon-core-test`
→ `ManifestFileTest` / `ManifestListTest` / `SnapshotFileScanTest` /
`FileUtilsTest` 37/37 passed; the scan suites (`*FileStoreScan*`, `*ScanTest*`,
`ManifestFileMergerTest`, `SnapshotManagerTest`) 99/99 passed. `paimon-fs-test`
→ Jindo and ListDir suites 14/14 passed. `git diff --check` clean.
### API and Format
Public API under `include/paimon/` is unchanged, and there is no storage
format or protocol change. The signatures that gained a parameter
(`ObjectsFile`, `ManifestFile::ReadBucketEntries`) are internal `src/paimon`
helpers, and the new `file_size` parameter is defaulted, so existing callers
are unaffected. The change relies on the existing `FileSystem::Open(const
FileStatus&)` contract, where the caller-supplied length is trusted rather than
re-validated; the manifest and manifest-list files it applies to are
write-once, so a length recorded at planning time cannot go stale underneath
the read.
### Documentation
No new table option or user-facing configuration key, and no new feature to
document. The optimization and the trusted-length rationale are documented in
code comments at `OpenForRead`, `ListVersionedFileStatus`, and Jindo `ListDir`.
### Generative AI tooling
Generated-by: Qoder
--
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]
