Skip to content

[core] Prepare PK vector index lifecycle for multiple columns - #10688

Open
tzphh wants to merge 1 commit into
apache:masterfrom
tzphh:feature/pk-multiple-vector-indexes
Open

tzphh wants to merge 1 commit into
apache:masterfrom
tzphh:feature/pk-multiple-vector-indexes

Conversation

@tzphh

@tzphh tzphh commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Related to #10658. Part 1 of the implementation series.

Prepare the primary-key index coordinator to manage multiple vector index
maintainers. Coordinate commit preparation, reverse-order rollback,
executor replacement, and resource cleanup across the maintainers.

Single-column configuration validation remains unchanged. Multiple-column
enablement and bounded parallel builds are covered by the following PRs.

Update the writer test to read the vector maintainer list and preserve
the deletion-vector factory assertion. This regression fix belongs in
this prerequisite commit so it can pass independently.

Tests

  • 19 focused Core tests passed on the updated CR1 head:
    PrimaryKeyIndexWriteTest, BucketedPrimaryKeyIndexMaintainerTest,
    and PrimaryKeyVectorIndexWriteTest.
  • Spotless, Checkstyle, Enforcer, and diff checks passed.
  • Local validation used JDK 17 and Maven 3.9.10; JDK 8/11 CI results
    remain separate from this local run.

@tzphh tzphh changed the title [core] Support multiple vector indexes on primary-key tables [wip][core] Support multiple vector indexes on primary-key tables Oct 20, 2026
@JingsongLi
JingsongLi marked this pull request as draft October 22, 2026 11:02
@tzphh
tzphh force-pushed the feature/pk-multiple-vector-indexes branch from 9f21d3d to fc6018f Compare October 22, 2026 13:42
@tzphh tzphh changed the title [wip][core] Support multiple vector indexes on primary-key tables [core] Prepare PK vector index lifecycle for multiple columns Oct 22, 2026
@tzphh
tzphh marked this pull request as ready for review October 22, 2026 13:58
public final class BucketedPrimaryKeyIndexMaintainer {

@Nullable private final BucketedVectorIndexMaintainer vectorMaintainer;
private final List<BucketedVectorIndexMaintainer> vectorMaintainers;

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.

Renaming this field breaks PrimaryKeyIndexWriteTest.testCreatesCoordinatorForMultipleScalarDefinitionsAndVector, which still calls readField(..., "vectorMaintainer") at line 142. Both Core JDK 8 and JDK 11 CI fail with NoSuchFieldException. Please read vectorMaintainers, assert that the list has one entry, and use its first maintainer for the existing deletion-vector-factory assertion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0479ef5. The test now reads vectorMaintainers, asserts that it contains one maintainer, and uses that maintainer for the existing deletion-vector factory assertion. All 19 focused Core tests passed locally on this CR1 head, including the previously failing test (JDK 17).

I also verified the combined head e787eb7, which includes this fix, on JDK 8 (1.8.0_512) and JDK 11 (11.0.27). All 139 focused Core tests passed on each runtime, including this regression test.

@JingsongLi

Copy link
Copy Markdown
Contributor

Requirement fit: SUPPORTED as the concrete lifecycle prerequisite for the implemented multi-column/parallel-build series. I found no additional production lifecycle defect in the own-head rollback, executor replacement, checkpoint/restore, or cleanup paths.

I independently reproduced the reflection-test issue already reported above on fc6018f: PrimaryKeyIndexWriteTest.testCreatesCoordinatorForMultipleScalarDefinitionsAndVector throws NoSuchFieldException: vectorMaintainer at line 142. Both Core CI jobs fail on this case. Please include the test correction in this prerequisite commit so each PR builds independently; the later parallel-build PR's correction does not fix this head.

Verification: 67 focused Core configuration, lifecycle, writer, compaction/reopen and vector-search tests passed with normal JDK 8 Maven verify. The failing reflection case was run separately and is still a merge blocker. The other inspected Flink 2/Iceberg CI failures are Maven Central HTTP 502 dependency downloads. No duplicate inline finding added.

@tzphh
tzphh force-pushed the feature/pk-multiple-vector-indexes branch from fc6018f to 0479ef5 Compare October 27, 2026 07:08
@tzphh

tzphh commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

The fix is now included in CR1 itself (0479ef5), and CR2/CR3 have been rebased on it. CR1 no longer depends on CR3 for this test correction. All 19 focused Core tests passed locally on this head, including PrimaryKeyIndexWriteTest (JDK 17).

For additional compatibility coverage, I tested the combined head e787eb7 on JDK 8 (1.8.0_512) and JDK 11 (11.0.27). All 139 focused Core tests passed on each runtime, including the previously failing reflection test.

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