Skip to content

[python] Build global index shards concurrently - #9798

Merged
JingsongLi merged 2 commits into
apache:masterfrom
TheR1sing3un:codex/global-index-shard-parallelism
Sep 17, 2026
Merged

JingsongLi merged 2 commits into
apache:masterfrom
TheR1sing3un:codex/global-index-shard-parallelism

Conversation

@TheR1sing3un

Copy link
Copy Markdown
Member

Purpose

PyPaimon's generic global-index builder currently builds vector and full-text shards one at a time. On multi-shard builds this leaves storage bandwidth and native index capacity idle.

Add global-index.build.parallelism (default 1) and use a bounded thread pool when the value is greater than one. Each worker keeps the existing streaming Arrow reader and one writer per shard, manifest messages are returned in shard-plan order, and a failed build closes active resources and removes every uncommitted index file. The documentation warns that each shard may also start native worker threads.

Benchmark

A temporary local benchmark exercised the production shard scheduler and streaming batch path with 4,000 approximately 2 KiB text rows per shard. It injected 20 ms of read latency plus 40 ms of native-build/upload latency per shard to isolate the scheduling change. Results are medians of three runs on macOS arm64; RSS is peak resident memory.

Shards Parallelism Time (s) Speedup Peak RSS (MiB)
4 1 0.448 1.00x 70.2
4 2 0.287 1.56x 99.6
4 4 0.207 2.16x 150.3
8 1 0.763 1.00x 70.2
8 2 0.456 1.68x 99.5
8 4 0.294 2.59x 157.7
8 8 0.238 3.21x 269.3
16 1 1.421 1.00x 70.2
16 2 0.783 1.82x 99.7
16 4 0.470 3.02x 157.8
16 8 0.339 4.20x 274.4

The ablation shows the throughput/memory tradeoff behind the conservative default. The benchmark uses simulated native and I/O latency, so end-to-end gains will depend on the index implementation, storage, shard size, and native thread settings. The temporary benchmark harness is not included in this change.

Tests

  • python -m pytest -q pypaimon/tests/global_index_build_test.py (36 passed)
  • python -m flake8 --config=dev/cfg.ini pypaimon/common/options/core_options.py pypaimon/globalindex/create_global_index.py pypaimon/tests/global_index_build_test.py

@TheR1sing3un
TheR1sing3un force-pushed the codex/global-index-shard-parallelism branch from 42134f1 to da87cce Compare September 14, 2026 04:19

@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. Implementation: FINDINGS.

Reviewed da87cce08afa. Building independent global-index shards concurrently has a real caller-visible throughput path with a conservative default. Normal worker failure cleanup and deterministic manifest ordering are covered, but output ownership is lost when task submission itself fails.

Validation: 46 tests and 32 subtests passed. A separate reproduction using the production scheduler, a real ThreadPoolExecutor, temporary output files and an injected second-worker startup failure left both uncommitted shard outputs behind. Current CI is green. Native throughput was not measured locally.

Comment thread paimon-python/pypaimon/globalindex/create_global_index.py Outdated
The parallel build assigned `futures` only after the whole comprehension
succeeded, so a submission that raised (for example `RuntimeError: can't
start new thread`) left it as the empty list. Shards accepted before the
failure still ran during executor shutdown, but rollback saw no futures
and left their index files behind.

Appending each future as it is returned is not enough: `submit()` puts the
work item on the queue before it starts an extra worker, so even the
submission that raises can run its shard on an already running worker.

Track output ownership in the worker itself so rollback no longer depends
on the future list, and cancel still-queued shards when submission fails.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@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.

Reviewed the output rollback ownership changes. Retaining rollback responsibility when a later executor submission fails prevents already-created outputs from being orphaned. The failure-path coverage looks good to me.

@JingsongLi
JingsongLi merged commit 772399a into apache:master Sep 17, 2026
12 of 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