[improvement](paimon) Support IOManager for JNI reads#65332
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
I found one correctness issue that should be fixed before merge: the IOManager tmp-dir fallback is only added in the legacy Paimon JNI reader, while the default FileScannerV2 Paimon JNI path still reaches Java without the tmp-dir option and fails for enable-only catalogs.
Critical checkpoint conclusions:
- Goal: partially met; IOManager support works on the changed legacy path but not on the default scanner-v2 path.
- Scope: the change is focused, but misses a parallel BE reader implementation.
- Concurrency/lifecycle: no new shared concurrency issue found; Java close handles reader and IOManager cleanup on normal/error paths.
- Configuration/session propagation: the opt-in catalog property is forwarded, but BE defaulting is incomplete across reader paths.
- Compatibility/persistence: no storage-format or edit-log compatibility issue found.
- Tests: helper/unit coverage was added, but it does not cover scanner v2 with
paimon.doris.enable_jni_io_manager=trueand no explicit tmp dir. - Validation: static review only in this runner because
thirdparty/installedandthirdparty/installed/bin/protocare missing.
Subagent conclusions: optimizer-rewrite proposed OPT-1 and tests-session-config proposed duplicate TSC-1; both were merged into the single inline comment. The plugin-driven Paimon connector path was dismissed because current catalog dispatch still routes paimon to the built-in catalog. Convergence round 1 ended with both subagents reporting NO_NEW_VALUABLE_FINDINGS for the final ledger/comment set.
User focus: no additional user-provided focus was present.
Gabriel39
force-pushed
the
codex/paimon-jni-io-manager
branch
from
a2b2223 to
8f8029e
Compare
Gabriel39
force-pushed
the
codex/paimon-jni-io-manager
branch
from
8f8029e to
b70411d
Compare
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Paimon JNI reads did not attach Paimon IOManager when creating TableRead, so primary-key merge reads in the Java scanner could not spill through Paimon IOManager. This change forwards catalog-level Paimon JNI IOManager options from FE to BE, creates and closes the IOManager in the Java scanner, supports an optional custom IOManager implementation class that is already available on the scanner classpath, and injects a default IOManager temp directory for both legacy and Format V2 JNI scan paths when the user enables IOManager without specifying a temp dir. The default temp directory is `${storage_root_path}/paimon_jni_scanner_io_tmp` for each BE storage root.
### Release note
Paimon JNI scanner can enable Paimon IOManager spill with catalog property `paimon.doris.enable_jni_io_manager=true`; `paimon.doris.jni_io_manager.tmp_dir` can optionally override the spill temp directory, and `paimon.doris.jni_io_manager.impl_class` can optionally select a custom `org.apache.paimon.disk.IOManager` implementation already available on the scanner classpath.
### Check List (For Author)
- Test: Unit Test / Manual test
- `mvn test -Dmaven.build.cache.enabled=false -pl be-java-extensions/paimon-scanner -Dtest=PaimonJniScannerTest` passed on `gabriel@10.26.20.3:/mnt/disk3/gabriel/Workspace/dev2/doris`.
- `python3 build-support/run_clang_format.py ...` check-only passed for modified BE C++ files.
- `git diff --check` passed.
- `ninja -C be/ut_build_ASAN src/format/CMakeFiles/Format.dir/table/paimon_jni_reader.cpp.o src/format/CMakeFiles/Format.dir/__/format_v2/jni/paimon_jni_reader.cpp.o` compiled the modified reader objects on the remote validation host.
- `./run-be-ut.sh --run --filter="PaimonJniReaderTest.*"` was attempted on the remote validation host but stopped before target tests due to unrelated missing header `faiss/invlists/PreadInvertedLists.h` while compiling `be/src/storage/index/ann/faiss_ann_index.cpp`.
- `./run-fe-ut.sh --run org.apache.doris.datasource.paimon.source.PaimonScanNodeTest` was attempted on the remote validation host but stopped before target tests due to unrelated generated `ScalarBuiltins` compile errors referencing missing `FunctionSet`.
- Behavior changed: Yes. When enabled, Paimon JNI reads now create a Paimon IOManager and use a storage-root scoped temp directory by default. Users can optionally choose a preinstalled custom IOManager implementation class.
- Does this need documentation: No. The PR description documents the catalog properties, default temp directory, and custom IOManager implementation behavior.
Gabriel39
force-pushed
the
codex/paimon-jni-io-manager
branch
from
b70411d to
c6bca61
Compare
Contributor
There was a problem hiding this comment.
Current head addresses the earlier blocking V2-path issue. I did not find another substantiated correctness issue that warrants a new inline comment.
Critical checkpoint conclusions:
- Goal: met for this patch. FE forwards the opt-in Paimon JNI IOManager catalog options, both legacy and FileScannerV2 JNI readers inject a storage-root scoped tmp-dir default for enable-only catalogs, and the Java scanner creates/closes the Paimon IOManager.
- Scope: focused on Paimon JNI IOManager option propagation, BE tmp-dir defaulting, Java scanner lifecycle, and targeted tests.
- Concurrency/lifecycle: no new shared concurrency issue found. The Java scanner closes reader and IOManager on normal close and after open/read failures; Paimon 1.3.1 creates/removes per-IOManager child directories under the configured parent temp dirs.
- Configuration/session propagation: scan-level
TFileScanRangeParams.paimon_optionsis populated from Paimon catalog properties and consumed by both legacy and V2 JNI paths. The prior scanner-v2 enable-only gap is now fixed. - Compatibility/persistence: no storage-format, edit-log, or wire-format compatibility issue found; existing scan-level
paimon_optionsthrift fields are reused. - Parallel paths: legacy JNI and FileScannerV2 JNI paths were checked. Native/C++ split paths are not enabled by these Doris-only IOManager keys.
- Tests: focused FE, Java scanner, and BE V2 reader unit coverage was added. I could not run tests in this runner because
.worktree_initialized,thirdparty/installed, andthirdparty/installed/bin/protocare missing, so validation here is static. - Observability/performance: the added Java-side IOManager counter is lightweight; no hot-path performance regression was substantiated.
Subagent conclusions: optimizer-rewrite and tests-session-config both found no new valuable candidates on the initial pass. The existing V2 fallback concern was treated as duplicate of discussion_r3536066774 and verified fixed in current head. Convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same ledger and empty final inline comment set.
User focus: no additional user-provided review focus was present.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
I re-reviewed the refreshed head c6bca61a45652fb229c554fd01d8f01b578c684c, including the force-pushed custom Paimon JNI IOManager implementation-class support. I did not find a new substantiated correctness issue that warrants an inline comment.
Critical checkpoint conclusions:
- Goal: met for this patch. FE forwards the opt-in Paimon JNI IOManager catalog options, both legacy and FileScannerV2 JNI readers inject a storage-root scoped tmp-dir default for enable-only catalogs, and the Java scanner creates/closes the Paimon IOManager.
- Scope: focused on Paimon JNI IOManager option propagation, BE tmp-dir defaulting, Java scanner lifecycle, optional custom IOManager implementation selection, and targeted tests.
- Concurrency/lifecycle: no new shared concurrency issue found. The Java scanner closes reader and IOManager on normal close and after open/read failures; the custom implementation path uses the created IOManager through the same close path.
- Configuration/session propagation: scan-level
TFileScanRangeParams.paimon_optionsis populated from Paimon catalog properties and consumed by both legacy and V2 JNI paths. The newpaimon.doris.jni_io_manager.impl_classoption is forwarded asdoris.jni_io_manager.impl_classfrom FE, then passed to Java aspaimon.doris.jni_io_manager.impl_class. - Compatibility/persistence: no storage-format, edit-log, or wire-format compatibility issue found; existing scan-level
paimon_optionsthrift fields are reused. - Parallel paths: legacy JNI and FileScannerV2 JNI paths were checked. Native/C++ split paths can receive the scan-level option map but do not enable Java IOManager behavior from these Doris-only JNI keys.
- Tests: focused FE, Java scanner, and BE V2 reader unit coverage was added, including forwarding of
impl_classand the Java customIOManagerconstruction path. I could not run tests in this runner because.worktree_initialized,thirdparty/installed, andthirdparty/installed/bin/protocare missing, so validation here is static. - Observability/performance: the Java-side IOManager counter remains lightweight; no hot-path performance regression was substantiated.
Subagent conclusions: after the head refresh and main ledger update, both live subagents reran convergence for the same empty inline comment set and replied NO_NEW_VALUABLE_FINDINGS. The existing V2 fallback concern remains duplicate of discussion_r3536066774 and is verified fixed in current head.
User focus: no additional user-provided review focus was present.
Contributor
|
Codex automated review failed and did not complete. Error: Codex completed, but no new pull request review was submitted for the current head SHA. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
There was a problem hiding this comment.
Reviewed the current head for Paimon JNI IOManager support across FE catalog option propagation, legacy and FileScannerV2 BE JNI reader parameter construction, Java scanner IOManager creation/close, and the added FE/BE/Java tests.
Critical checkpoint conclusions:
- Goal and correctness: the PR wires the catalog-level Paimon JNI IOManager options to the backend, creates the IOManager before
TableRead.withIOManager(...), and closes it on normal close and open/read failure paths. The current head also covers the FileScannerV2 Paimon JNI path, so the earlier inline concern about missing V2 fallback appears addressed. - Scope: the change is focused on Paimon JNI scan option propagation and scanner lifecycle, with related tests only.
- Concurrency and lifecycle: I did not find new shared mutable concurrency hazards. The Java scanner owns one reader/IOManager per scanner instance and clears them during close; open failure calls close and suppresses close failures onto the original exception.
- Configuration and compatibility: the new catalog properties are passed through as backend Paimon options without changing persisted formats or FE/BE thrift schema. The BE injects a storage-root scoped default temp dir when IOManager is enabled without an explicit dir.
- Parallel paths: both legacy
PaimonJniReaderand Format V2format_v2/jni/PaimonJniReadernow apply the enable-only temp-dir fallback. The C++/native Paimon path may receive the same scan-level options but does not use this IOManager behavior. - Tests: added coverage checks FE backend option extraction, scan-level option application, V2 scanner parameter fallback, Java default/custom IOManager creation, and IOManager close cleanup. I did static review in this runner and a changed-file whitespace check; I did not run the full unit test suites here.
- User focus: no additional user-provided review focus was supplied.
Subagent conclusions:
optimizer-rewrite: no candidates became inline comments; final convergence response wasNO_NEW_VALUABLE_FINDINGS.tests-session-config: no candidates became inline comments; final convergence response wasNO_NEW_VALUABLE_FINDINGS.- Existing thread 3536066774 was treated as already-known review context and not duplicated; the current head contains the requested V2 fallback and test coverage.
I found no new blocking issue to submit as an inline review comment.
Contributor
Author
|
run buildall |
2 similar comments
Contributor
Author
|
run buildall |
Contributor
Author
|
run buildall |
Contributor
Author
|
run beut |
Contributor
Author
|
run feut |
Contributor
Author
|
run performance |
1 similar comment
Contributor
Author
|
run performance |
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29184 ms |
Contributor
TPC-DS: Total hot run time: 179737 ms |
Contributor
ClickBench: Total hot run time: 25.12 s |
Contributor
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 29475 ms |
Contributor
TPC-DS: Total hot run time: 179547 ms |
Contributor
ClickBench: Total hot run time: 25.12 s |
yiguolei
approved these changes
Jul 8, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
Collaborator
|
skip buildall |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Paimon JNI reads created
TableReadwithoutwithIOManager, so Paimon primary-key merge reads could not spill through Paimon IOManager. This PR wires Doris catalog properties through FE and BE into the Java scanner, creates the Paimon IOManager for JNI reads, and closes it on scanner close or open failure.The same IOManager option/default temp-dir handling is implemented in both Paimon JNI scan paths:
be/src/format/table/paimon_jni_reader.cppbe/src/format_v2/jni/paimon_jni_reader.cppIssue Number: None
How to configure
Users enable the Paimon JNI IOManager on the Paimon catalog:
Users can optionally set the spill temp directory explicitly:
If
paimon.doris.jni_io_manager.tmp_diris not set, Doris uses${storage_root_path}/paimon_jni_scanner_io_tmpas the default parent directory. When a BE has multiplestorage_root_pathentries, Doris passes all corresponding directories to Paimon IOManager, joined by the platform path separator, for example/data1/doris/paimon_jni_scanner_io_tmp:/data2/doris/paimon_jni_scanner_io_tmpon Linux. The Java scanner creates these parent directories when opening IOManager; Paimon creates and removes its ownpaimon-*child directory under them.IOManager implementation
Doris delegates IOManager behavior to the Java scanner. By default, the scanner calls
org.apache.paimon.disk.IOManager.create(...). With the Paimon 1.3.1 dependency used by Doris, that factory creates Paimon's defaultorg.apache.paimon.disk.IOManagerImpl, which lazily creates aFileChannelManagerImpl, createspaimon-io-*child directories under the configured temp directories, and removes those child directories when the IOManager is closed.Users can optionally select a custom IOManager implementation class with a catalog property:
The custom class must already be available on the BE Paimon scanner classpath and must implement Paimon's Java interface
org.apache.paimon.disk.IOManager. Doris does not load arbitrary jar URLs from this property; deployment of the custom implementation jar is an operator action. The scanner instantiates the class with the first public constructor it finds in this order:CustomIOManager(String[] tempDirs)CustomIOManager(String tempDirs)CustomIOManager()After instantiation, the scanner passes it to
TableRead.withIOManager(customIOManager).Release note
Paimon JNI scanner can enable Paimon IOManager spill with catalog property
paimon.doris.enable_jni_io_manager=true;paimon.doris.jni_io_manager.tmp_dircan optionally override the spill temp directory, andpaimon.doris.jni_io_manager.impl_classcan optionally select a customorg.apache.paimon.disk.IOManagerimplementation already available on the scanner classpath.Check List (For Author)
PaimonJniReaderTest.BuildScannerParamsInjectsStorageRootTmpDirForEnabledIOManagerto cover the Format V2 enable-only IOManager path.PaimonJniScannerTest.testCreateDefaultAndCustomIOManagerto cover default and custom IOManager instantiation.mvn test -Dmaven.build.cache.enabled=false -pl be-java-extensions/paimon-scanner -Dtest=PaimonJniScannerTestpassed ongabriel@10.26.20.3:/mnt/disk3/gabriel/Workspace/dev2/doris.python3 build-support/run_clang_format.py ...check-only passed for modified BE C++ files.git diff --checkpassed locally and for the modified test file on the remote validation host.ninja -C be/ut_build_ASAN src/format/CMakeFiles/Format.dir/table/paimon_jni_reader.cpp.o src/format/CMakeFiles/Format.dir/__/format_v2/jni/paimon_jni_reader.cpp.ocompiled the modified reader objects on the remote validation host../run-be-ut.sh --run --filter="PaimonJniReaderTest.*"was attempted on the remote validation host but stopped before target tests due to unrelated missing headerfaiss/invlists/PreadInvertedLists.hwhile compilingbe/src/storage/index/ann/faiss_ann_index.cpp../run-fe-ut.sh --run org.apache.doris.datasource.paimon.source.PaimonScanNodeTestwas attempted on the remote validation host but stopped before target tests due to unrelated generatedScalarBuiltinscompile errors referencing missingFunctionSet.