lucasfang opened a new pull request, #332: URL: https://github.com/apache/paimon-cpp/pull/332
# PR Description > Template: `.github/PULL_REQUEST_TEMPLATE.md` ## PR Title perf(fs): skip getFileStatus on Jindo open when the file length is known ### Purpose Linked issue: close #331 The read path opens data files with a length it already knows — the manifest carries the file size, and both `DataFileReaderFactory::Open` and `PrefetchFileBatchReaderImpl` pass `FileStatus(path, size)` to `FileSystem::Open`. But `JindoFileSystem` only implemented `Open(path)`, and the base `Open(const FileStatus&)` default forwards to it, so the known length was dropped and the Jindo store issued its own `getFileStatus` RPC to resolve the size on every open — one extra full-latency round-trip to OSS per file open, per reader. The arrow-based `ObjectStoreFileSystem` already avoids this via its `Open(const FileStatus&)`; this brings the Jindo backend in line. Changes: - `src/paimon/fs/jindo/jindo_file_system.h` / `.cpp`: override `Open(const FileStatus&)`, validate the length is non-negative, and route both `Open` overloads through a new protected `OpenReader(path, std::optional<int64_t> file_length)` hook. `Open(path)` passes `std::nullopt` and keeps the original behavior. - `third_party/jindosdk-nextarch/include/JdoFileSystem.hpp` / `src/JdoFileSystem.cpp`: add an `openReader(path, file_length, result)` overload that sets `JDO_OPEN_OPTS_HAS_GET_FILE_STATUS=true` and `JDO_OPEN_OPTS_FILE_LENGTH=<len>` on the open options. The `HAS_GET_FILE_STATUS` hint is the gate that suppresses the store's `getFileStatus`; `FILE_LENGTH` supplies the trusted size. The benefit is automatic for the existing read path, since it already passes a `FileStatus`. ### Tests - UT `src/paimon/fs/jindo/jindo_file_system_unit_test.cpp` (4 new cases in `JindoFileSystemUnitTest`): `OpenWithoutFileStatusLeavesLengthToStore` (no status → `std::nullopt`), `OpenWithFileStatusPassesTrustedLength` (length forwarded, path preserved), `OpenWithFileStatusKeepsZeroLengthOnFastPath` (zero length still uses the fast path), `OpenWithFileStatusRejectsNegativeLength` (negative length rejected before any open). Adds a `TestInputStream` and records the paths/lengths passed to `OpenReader`. - Actual results: `cmake --build build-debug-bench --target paimon-fs-test -j 96` built clean; `paimon-fs-test --gtest_filter='JindoFileSystemUnitTest.*'` → 7/7 passed; the offline suites (`FileSystemStaticTest`, `FsType/FileSystemTest`, `JindoFileSystemUnitTest`, `JindoUtilsTest`, `LocalFileTest`, `ResolvingFileSystemTest`) → 122 ran, 117 passed, 5 skipped (pre-existing environment skips), 0 failed. `git diff --check` clean. The OSS network suites (`JindoFileSystemTest`, `JindoFileSystemFactoryTest`, `JindoFileSystemAsyncReadTest`, `JindoFileSystemPaginationTest`) were not run here — they require real OSS credentials. ### API and Format Public API: unchanged. `FileSystem::Open(const FileStatus&)` already existed in `include/paimon/fs/file_system.h`; this only implements it in the `JindoFileSystem` subclass and adds an internal protected `OpenReader` hook. The new `JdoFileSystem::openReader` overload lives in vendored `third_party/` and is not a public Paimon header. Storage format and protocol: unchanged — this only affects how a reader is opened, not what is read or written. ### Documentation No new table option or user-facing configuration key. The behavior is an internal open-path optimization; the trusted-length contract is documented in code comments on `OpenReader` and the new `openReader` overload. ### 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]
