Skip to content

fix(python): rebuild the fallback runtime after fork - #832

Merged
JingsongLi merged 2 commits into
apache:mainfrom
JingsongLi:codex/fix-python-fork-runtime
Sep 15, 2026
Merged

JingsongLi merged 2 commits into
apache:mainfrom
JingsongLi:codex/fix-python-fork-runtime

Conversation

@JingsongLi

Copy link
Copy Markdown
Contributor

Purpose

Fix native planning hanging in forked Python workers after the parent has used a Paimon catalog or scan. A Torch DataLoader with num_workers=2 hits this in apache/paimon#9825: the child inherits the global Tokio runtime but none of its worker threads, so catalog I/O and planning never finish.

Brief change log

  • Associate the lazily created fallback runtime with its process ID and publish its state through an atomic pointer.
  • After fork, initialize a runtime for the child without acquiring a potentially inherited initialization lock or dropping the parent's runtime state. Published states remain allocated because dropping the inherited runtime could wait for threads that no longer exist.
  • Preserve the existing behavior of using a Tokio runtime already entered on the calling thread.

Tests

  • Two real Python fork regressions: create a new catalog or reuse the parent's catalog, plan and serialize splits twice in the child, and confirm the parent can still plan. Both timed out before the fix and pass with it; each has bounded waiting and child cleanup.
  • Concurrent fallback-runtime reuse and preservation of an entered runtime: 2 Rust unit tests passed.
  • Python binding fork/read/table/write/catalog tests: 105 passed.
  • PyPaimon validation with the rebuilt wheel: the explicit-fork DataLoader reproduction completes; 25 contiguous-window dataset tests and 357 planner/reader regressions pass with native planning.
  • cargo clippy --offline -p paimon-datafusion --all-targets -- -D warnings, cargo fmt --all -- --check, Python test Flake8, and git diff --check passed.

API and Format

No public API or storage-format changes. This fixes the fallback runtime used by catalog and planning calls; explicitly entered Tokio runtimes retain their existing behavior.

Documentation

Updated runtime comments to explain process ownership and why inherited state is retained.

@leaves12138 leaves12138 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed 386ef9345021064c2a91aa36d3b4a9e28a6cb510. No blocking findings.

The PID check and publication of a fresh, uninitialized state let the child bypass both an inherited runtime and an inherited in-progress OnceLock. The acquire/release ordering and the ownership of unsuccessful CAS allocations look correct; published parent state remains allocated, avoiding shutdown of inherited worker state. The existing entered-runtime behavior is preserved.

Local validation:

  • Compiled the exact runtime.rs standalone against cached Tokio dependencies: both unit tests passed.
  • An independent real-fork harness compiled against the base runtime hit the child's five-second deadline; the same harness passed against this head. It exercises child/grandchild processes, concurrent runtime reuse, TCP I/O, timers, the blocking pool, and continued parent use.
  • A deterministic fork while another thread holds the published state's initialization OnceLock also passed against this head.
  • cargo fmt --all -- --check and syntax parsing of the new Python test passed.

I reviewed the two Python fork regressions but did not rebuild the Python binding or execute the full Python/DataFusion integration suites locally.

Update rustls to 0.23.45 and its required AWS-LC/webpki dependencies.
Regenerate workspace dependency reports without suppressing the advisory.
@leaves12138

Copy link
Copy Markdown

Fixed the failing CI dependency-policy check in 0891c50d91768af2261e015582a2317ca73553a2.

cargo-deny rejected rustls 0.23.42 for RUSTSEC-2026-0285. Upgraded to rustls 0.23.45 and its required AWS-LC/webpki dependencies, and regenerated the nine dependency reports. No advisory suppression or fork-runtime changes.

Local validation passed: advisories/licenses against a freshly fetched RustSec database, dependency-report verification, all 21 generated release legal files, 12 release-tool tests, compilation of rustls 0.23.45, formatting, and staged-diff checks. The legal files were regenerated but needed no content changes.

The fix is appended to the existing PR branch without rewriting history. CI run 34920232610 has started and is still in progress.

@JingsongLi
JingsongLi merged commit 7fd9bab into apache:main Sep 15, 2026
14 checks passed
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.

2 participants