Skip to content

feat(storage): support Jindo for OSS scan planning - #696

Open
XiaoHongbo-Hope wants to merge 3 commits into
apache:mainfrom
XiaoHongbo-Hope:codex/jindo-storage
Open

XiaoHongbo-Hope wants to merge 3 commits into
apache:mainfrom
XiaoHongbo-Hope:codex/jindo-storage

Conversation

@XiaoHongbo-Hope

@XiaoHongbo-Hope XiaoHongbo-Hope commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Add an opt-in, read-only JindoSDK backend for OSS scans. OpenDAL remains the default. PyPaimon can select it for native planning through apache/paimon#9124.

Changes

  • add the optional storage-jindo feature, selected with fs.oss.impl=jindo
  • dynamically load an installed JindoSDK without bundling its binary
  • preserve HTTP/HTTPS endpoint protocol when adapting Jindo configuration
  • support stat, paginated list, and ranged/full reads
  • fetch list pages on demand and convert Jindo list exceptions into Rust errors
  • initialize lazily and run blocking SDK calls on Tokio blocking workers
  • bound blocking Jindo reads with fs.jindo.max.concurrent.reads (default 8 per OSS operator)
  • keep a read permit until the blocking C SDK call returns, including after async caller cancellation
  • reject unsupported write operations explicitly

Benchmark

Release-mode, partition-filtered scan planning on two fixed OSS-backed snapshots. This reads Paimon metadata and builds splits; it does not read data-file contents. Both backends ran on the same host with identical predicates, in reversed order and two independent processes per backend.

Workload Resulting plan OpenDAL planning time (warm median) Jindo planning time (warm median)
A ~281k data-file references 1.55s 1.25s
B ~37k data-file references / ~27k splits 5.92s 2.19s

The file counts are references in the resulting scan plan, not individual data files opened or read. Both backends produced matching snapshot, split, file-count, and planned-byte results. Request count and peak OSS QPS were not measured.

Data read benchmark

Release-mode reads on two fixed OSS-backed snapshots. Planning completed before the timer started. Each backend ran in three independent processes on the same host, in alternating order.

Workload Read result OpenDAL data-read time (warm median) Jindo data-read time (warm median)
A indexed descriptor point read: 1 row / 313 B Arrow output 1.09s 1.26s
B two-column read from one data file: 2,242,216 rows / 44.9 MiB Arrow output 0.49s 0.59s

Both backends produced matching row counts, Arrow sizes, and content checksums. Planning time is excluded. Network bytes and request count were not measured, so these results show end-to-end reader latency for these workloads, not raw OSS throughput.

These isolated serial reads were 16–20% slower with Jindo. A separate same-host 8×8 random-read A/B found lower normal-path read latency with Jindo, so the result is workload-dependent; the gate improves stability, not single-read overhead.

Read concurrency trade-off

A separate fixed single-node random-read workload used official pyjindosdk==6.10.401, 8 ranks × 8 workers, 16 samples per batch, 64 warm-up batches, and 4,096 measured samples per round.

Configuration Measured throughput Warm-up Jindo warnings
no Jindo read gate, four rounds 267.388 / 266.481 / 268.879 / 267.838 samples/s 14.595 / 14.219 / 14.385 / 103.819s 4,357 connect, 4 request
limit 8, two rounds 265.806 / 263.424 samples/s 9.126 / 9.463s 148 connect

The observed steady-throughput cost of limit 8 was about 1.1%. It substantially reduced connection warnings, and the reproduced 100-second warm-up tail did not recur in these two bounded rounds. Two rounds are not a long-term stability proof. In a separate limit-8 diagnostic soak, one Jindo C SDK getObject still blocked for about 60 seconds; therefore the gate limits request fan-out but does not fix JindoSDK DNS, connection-pool, or request-timeout defects.

A limit-4 tuning run produced 255.766 / 261.487 samples/s with zero connection warnings. It is more conservative but is marginal for a 260 samples/s target. The limit is per OSS operator, not process-global or host-global, so multiple processes/operators still multiply aggregate concurrency.

OpenDAL remains the default. Select Jindo only when its planning benefit or workload-level throughput has been validated together with tail latency and storage-request pressure for the deployment.

Validation

  • Jindo configuration tests and Python SDK discovery test
  • read-gate default/override/invalid-option tests
  • gate concurrency and cancellation-lifetime tests
  • Linux CI loads pyjindosdk==6.10.4 and exercises stat, paginated listing, ranged reads, 404/503 mapping, and C++ list exceptions against a local OSS-protocol server
  • default OSS and storage-jindo builds
  • Clippy, formatting, dependency report, and diff checks
  • read-only OSS REST-catalog test covering snapshot listing, planning, and row reading
  • JindoSDK 6.10.4 local OSS-protocol probes: HTTPS emits TLS, list exceptions return errors, three-page listing is complete, and consuming one entry does not request later pages

Notes

No JindoSDK binary is packaged. Selecting Jindo without the feature or a loadable SDK returns a configuration error. A FileIO configured with Jindo rejects writes, deletes, renames, and copies.

@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as ready for review August 9, 2026 04:01
@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as draft August 9, 2026 04:44
@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as ready for review August 9, 2026 06:20
@JingsongLi

Copy link
Copy Markdown
Contributor

Have you compared Jindo and OpenDAL? What are the differences? (benchmark)

@XiaoHongbo-Hope

Copy link
Copy Markdown
Contributor Author

Have you compared Jindo and OpenDAL? What are the differences? (benchmark)

yes, I did a test, Jindo performance is a little better, but there is a lot of qps issue during my testing, so may need another testing. And, I think this PR is best to have, not very urgent now. Because qps issue is addressed in other way.

@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as draft August 10, 2026 01:29
@XiaoHongbo-Hope
XiaoHongbo-Hope force-pushed the codex/jindo-storage branch 2 times, most recently from 258cf5f to 74ead67 Compare September 20, 2026 12:54
@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as ready for review September 20, 2026 13:34
@XiaoHongbo-Hope

Copy link
Copy Markdown
Contributor Author

Have you compared Jindo and OpenDAL? What are the differences? (benchmark)

I updated the performance result in PR descrption.

@XiaoHongbo-Hope
XiaoHongbo-Hope requested review from JingsongLi and QuakeWang and removed request for JingsongLi September 20, 2026 13:45

@QuakeWang QuakeWang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as draft September 21, 2026 04:17
@XiaoHongbo-Hope
XiaoHongbo-Hope marked this pull request as ready for review September 21, 2026 05:44
@JingsongLi

Copy link
Copy Markdown
Contributor

Requirement fit: SUPPORTED. Implementation: FINDINGS, reviewed at 45e887beb14cd1f67b1ae3ca5bfb10b26fdb21bc.

The opt-in planning backend has a concrete consumer in the merged apache/paimon#9124, and the supplied measurements demonstrate an end-to-end planning benefit for the two tested snapshots. Keeping the claim scoped to planning is appropriate: the reported data reads are slightly slower and request/QPS effects remain unmeasured.

[P2] Preserve explicit SDK environment selection before Python auto-discovery. In bindings/python/src/context.rs:122-124, a user who sets JINDOSDK_LIBRARY_PATH (or JINDOSDK_HOME) and has pyjindo installed gets fs.jindo.library.path injected from the Python package. library_candidates then treats that injected value as an exclusive explicit path and never consults the user's environment. Thus the documented SDK selection works for Rust callers but selects a different SDK through Python; if the package's library is not loadable, even a valid environment-selected SDK cannot be used. Please skip package discovery when either explicit SDK environment setting is present, or make package discovery a fallback after those settings, and cover this precedence in a binding test.

I traced configuration, lazy initialization, paginated listing, ranged reads, read-only operations, and the FileIO callers. cargo +1.94.0 test -p paimon --features storage-jindo --lib storage_jindo passes all 6 tests locally. I did not rerun the native SDK/OSS probes or the reported benchmarks; these unit tests do not validate the dynamic ABI or live storage behavior.

user: DEFAULT_JINDO_USER.to_string(),
properties: HashMap::new(),
};
let operator = jindo_config_build(&config, "bucket").unwrap();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a test that actually loads the SDK and covers stat, paginated listing, and ranged reads? The current tests stop before exercising those paths, so regressions there would not be caught.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requirement fit: SUPPORTED. The opt-in backend has a native planning consumer and the reported same-snapshot measurements show a planning benefit. The claim should remain scoped to planning; the reported data reads are slower and OSS request/QPS effects were not measured.

Implementation: CLEAN at 1975653c9129f18582738d907c53e7197df1f2a9 for the code paths I could verify. The latest commit fixes my earlier SDK path precedence finding: explicit catalog and JINDOSDK_* settings now take precedence over Python package discovery. I traced selection, lazy initialization, stat/list/range handling, read-only behavior, and cleanup again. I found no new actionable code defect.

Verification: cargo test -p paimon --features storage-jindo --lib storage_jindo passed (6 tests); cargo test -p pypaimon_rust --lib test_pyjindo_discovery_precedence passed; the current head's CI succeeded. Production validation remains pending: there is no Jindo SDK installed in this review environment, and the committed tests still do not load one. The SDK-backed stat, paginated listing, ranged read, and failure cases raised in QuakeWang's current review should be exercised before treating this backend as production-ready. The PR description's local probes are useful evidence, but they are not a repeatable regression gate.

@XiaoHongbo-Hope
XiaoHongbo-Hope force-pushed the codex/jindo-storage branch 2 times, most recently from 7a19ae7 to 48bd543 Compare September 26, 2026 12:22

@QuakeWang QuakeWang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@XiaoHongbo-Hope

Copy link
Copy Markdown
Contributor Author

Added a bounded Jindo read gate in bf2c86b:

  • new fs.jindo.max.concurrent.reads option, default 8 per OSS operator
  • the permit is held until the blocking Jindo C call actually returns, including when the async waiter is cancelled
  • invalid/non-positive limits fail configuration

Motivation from a fixed single-node 8-rank/8-worker read workload with official pyjindosdk 6.10.401:

Build Measured throughput Warm-up Jindo warnings
#696 without gate, four rounds 267.388 / 266.481 / 268.879 / 267.838 samples/s 14.595 / 14.219 / 14.385 / 103.819 s 4,357 connect, 4 request
same workload with limit 8, two rounds 265.806 / 263.424 samples/s 9.126 / 9.463 s 148 connect

The observed throughput cost was about 1.1%, while connection warnings dropped substantially and the reproduced warm-up tail did not recur in the bounded runs. This gate limits request fan-out; it does not replace an independent JindoSDK DNS/connection-pool fix.

Local validation: cargo test -p paimon --features storage-jindo storage_jindo --lib (9 passed, 1 existing SDK-dependent test ignored), Clippy with -D warnings, rustfmt, and diff checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants