Skip to content

[core] Snapshot sequence initialization drops existing HASH indexes - #11129

Open
taoran92 wants to merge 2 commits into
apache:masterfrom
taoran92:fix/snapshot-init-hash-index
Open

taoran92 wants to merge 2 commits into
apache:masterfrom
taoran92:fix/snapshot-init-hash-index

Conversation

@taoran92

@taoran92 taoran92 commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Purpose

Fixes #11128.

With write-only=true and write.sequence-number-init-mode=snapshot, restarting a dynamic-bucket writer can replace an existing HASH index with only the current write's hashes. Later updates can then reach a different bucket and leave duplicate primary keys in query results.

Restore indexes independently of the data-file metadata fast path, retain data metadata required by primary-key indexes, and preserve explicit overwrite semantics. Pass the metadata-scan flag through the Flink write coordinator as well.

Tests

  • Core regressions: 22 tests passed, covering HASH retention, bucket assignment, overwrite, sequence initialization, and file-backed deletion-vector/primary-key index restoration.
  • TableWriteCoordinatorTest: 9 tests passed, including paginated index restoration with and without data-file metadata scanning.

@taoran92
taoran92 force-pushed the fix/snapshot-init-hash-index branch from 47dce77 to 1e287ed Compare November 13, 2026 03:01
@taoran92
taoran92 force-pushed the fix/snapshot-init-hash-index branch from 1e287ed to dc8b31f Compare November 13, 2026 05:03
@taoran92
taoran92 marked this pull request as ready for review November 13, 2026 06:14

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

The HASH/index restoration fix addresses a real duplicate-primary-key failure. Normal verification passed 59 core tests and the 9 coordinator tests on both Flink 1 / JDK 8 and Flink 2 / JDK 17. Exact old/new coordination-request serialization probes also passed. One extension compatibility issue remains below.

boolean scanDeleteVectorsIndex,
boolean scanSourceIndexPayloads);
boolean scanSourceIndexPayloads,
boolean scanDataFileMetas);

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.

[P2] Preserve the existing WriteRestore extension method

Replacing the five-argument method with a new abstract six-argument method breaks existing WriteRestore implementations supplied through withWriteRestore, including thin wrappers around the built-in restore. When the new AbstractFileStoreWrite calls such a precompiled implementation, it throws AbstractMethodError before restoring the bucket. Existing binaries that invoke the former five-argument entry point instead throw NoSuchMethodError. I reproduced both directions on JDK 8, and confirmed an actual downstream compaction wrapper still implements/calls the former signature.

Please retain a compatible five-argument entry point and bridge the new overload (older implementations can safely keep scanning data metadata), or explicitly coordinate the consumer migration before rollout. The backward-compatible serialized request field does not cover this Java extension-method change.

@taoran92 taoran92 Nov 13, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@JingsongLi Thanks for reviewing! agreed. I have restored the five-argument overload and added a default bridge for compatibility, with regression tests for legacy wrappers. PTAL.

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.

[Bug] Snapshot sequence initialization can produce duplicate primary keys

2 participants