Skip to content

[core] Support bounded parallel builds for primary-key indexes - #10756

Open
tzphh wants to merge 3 commits into
apache:masterfrom
tzphh:feature/pk-multiple-vector-indexes-cr3
Open

tzphh wants to merge 3 commits into
apache:masterfrom
tzphh:feature/pk-multiple-vector-indexes-cr3

Conversation

@tzphh

@tzphh tzphh commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Related to #10658. Part 3 of the implementation series.
Depends on #10688 and #10755.

Add pk-index.build.parallelism (default: 1) to support bounded parallel
builds for Vector, Full Text, BTree, Bitmap, Multivalue, and FM indexes.
All configured index columns and buckets within a writer share one executor.

Start index builds before waiting, merge prepared changes only after all
indexes succeed, and roll back both preparation rounds in reverse order
on failure. Remove the single-active-sorted-index scheduling restriction
and retire stale vector coverage before non-blocking preparation returns.

This branch includes both prerequisite commits. For the changes specific
to this PR, review commit e787eb7f96.
The branch will be rebased after the prerequisites are merged.

Synchronize builder startup in the scalar-failure and interruption tests
before triggering failure or interruption. Clean up the commit thread
and blocked build even if an assertion fails.

Tests

  • 139 Core tests passed on the final source without the temporary
    worker-delay harness, including writer regression, multi-column search,
    lifecycle, rollback, and mixed-index maintenance coverage.
  • Both reported races reproduced before the fix with controlled worker
    startup delays; both targeted tests passed with the same delays after
    adding startup latches. Original execution-count and output-file
    assertions are preserved. The delay harness is not part of this PR.
  • Spotless, Checkstyle, Enforcer, and diff checks passed. Apache RAT passed
    on a clean export of the final tracked source.
  • Local validation used JDK 17 and Maven 3.9.10. Native ANN performance
    and JVM/Python interoperability were not tested in this run.

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

The new concurrent first preparation round exposes a race in the existing testBlockingPrepareRollsBackAllDefinitionsAfterInterruption test. The test waits only for secondStarted before interrupting, so the first build may still be queued and can be cancelled before execution. In that case the retry leaves firstBuilds == 1, although the test requires 2. I observed this failure once in the focused suite, while eight isolated reruns passed. Could we also wait for a firstStarted latch before interrupting, or make the assertions account for cancellation before execution?

commitThread.interrupt(); on line 674 - can race with the first build actually starting

@JingsongLi

Copy link
Copy Markdown
Contributor

[P2] Synchronize task startup before asserting build execution

Location: BucketedPrimaryKeyIndexMaintainerTest.java:538

Submitting both builds does not guarantee that both have started. If the second worker is delayed, BTree can exhaust its three attempts and fail before the bitmap task begins. Rollback then correctly cancels that task before execution, leaving buildOrder as ["btree", "btree", "btree"], so this new assertion fails even though cancellation works correctly. I reproduced this deterministically with the same two-thread executor by delaying only the second worker's startup.

Please either add a bitmapStarted latch and make the failing BTree build wait for it, or allow the assertion to account for cancellation before execution. The interruption test has the same scheduling assumption: waiting only for secondStarted does not establish that the first build has started, so its later firstBuilds == 2 assertion can also fail; delaying the first worker reproduces firstBuilds == 1 after retry.

The 76 focused Core tests passed under normal scheduling; both failures above were reproduced with controlled worker scheduling.

@tzphh
tzphh force-pushed the feature/pk-multiple-vector-indexes-cr3 branch from e9c1078 to e787eb7 Compare October 27, 2026 07:08
@tzphh

tzphh commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

The new concurrent first preparation round exposes a race in the existing testBlockingPrepareRollsBackAllDefinitionsAfterInterruption test. The test waits only for secondStarted before interrupting, so the first build may still be queued and can be cancelled before execution. In that case the retry leaves firstBuilds == 1, although the test requires 2. I observed this failure once in the focused suite, while eight isolated reruns passed. Could we also wait for a firstStarted latch before interrupting, or make the assertions account for cancellation before execution?

commitThread.interrupt(); on line 674 - can race with the first build actually starting

Fixed in e787eb7 by adding firstStarted and waiting for both builders to start before interrupting. The two-build and btree-2 / bitmap-2 assertions are unchanged. Delaying the first worker by one second reproduced the failure before the fix and passed afterward with the same delay.

After removing the temporary delay harness, all 139 focused Core tests passed locally on this head on JDK 8 (1.8.0_512), JDK 11 (11.0.27), and JDK 17, including the interruption/rollback test.

@tzphh

tzphh commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

[P2] Synchronize task startup before asserting build execution

Location: BucketedPrimaryKeyIndexMaintainerTest.java:538

Submitting both builds does not guarantee that both have started. If the second worker is delayed, BTree can exhaust its three attempts and fail before the bitmap task begins. Rollback then correctly cancels that task before execution, leaving buildOrder as ["btree", "btree", "btree"], so this new assertion fails even though cancellation works correctly. I reproduced this deterministically with the same two-thread executor by delaying only the second worker's startup.

Please either add a bitmapStarted latch and make the failing BTree build wait for it, or allow the assertion to account for cancellation before execution. The interruption test has the same scheduling assumption: waiting only for secondStarted does not establish that the first build has started, so its later firstBuilds == 2 assertion can also fail; delaying the first worker reproduces firstBuilds == 1 after retry.

The 76 focused Core tests passed under normal scheduling; both failures above were reproduced with controlled worker scheduling.

Both scheduling assumptions are fixed in e787eb7. The BTree builder waits for bitmapStarted before failing, and the interruption test waits for both builders to start. The existing execution-count, rollback-state, and output-file assertions are preserved. Both failures were reproduced with delayed worker startup and passed with the same delays after the fix.

After removing the temporary harness, all 139 focused Core tests passed locally on this head on JDK 8 (1.8.0_512), JDK 11 (11.0.27), and JDK 17, including both rollback/retry tests.

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