You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "leaves12138 (via GitHub)" <gi...@apache.org> on 2026/05/26 06:01:19 UTC
[PR] [core][flink] Detect row-id reassign conflicts for index commits [paimon]
leaves12138 opened a new pull request, #7972:
URL: https://github.com/apache/paimon/pull/7972
### Purpose
`row-id` reassignment rewrites row-id ranges for data evolution tables. If a global index build or index compact job plans work before reassignment and commits after reassignment, it can commit stale index metadata for the old row ids.
This PR adds a snapshot-level `row-id-reassign=true` marker and makes row-id-sensitive index commits fail fast when such a marker appears after the snapshot they planned from.
### Changes
- Add `Snapshot.ROW_ID_REASSIGN_PROPERTY`.
- Add row-id-reassign conflict detection in `ConflictDetection` and wire it through `FileStoreCommit` / table commit implementations.
- Record the scan snapshot used by btree and generic global index builders.
- Pass the planned scan snapshot into btree and generic global index commits.
- Strip the transient reassignment marker from normal manifest rewrite/compact snapshots.
- Add focused tests for detecting reassignment conflicts and preserving/removing snapshot properties.
### Validation
- `mvn -pl paimon-core -Dtest=ConflictDetectionTest,FileStoreCommitTest test`
- `mvn -pl paimon-flink/paimon-flink-common -Dtest=BTreeIndexTopoBuilderTest test`
- Preprod validation with a data evolution table:
- created a 48-partition table state with 1,177,600 rows and incremental btree index holes;
- ran incremental btree index build and row-id reassignment concurrently;
- confirmed reassignment snapshot carried `row-id-reassign=true`;
- confirmed stale index commit fails with:
`Row-id reassignment snapshot 904 was committed after the task planned from snapshot 903. The task must be retried with the latest row ids.`
- retried index build after reassignment and verified `MISSING_INDEX=0` plus indexed point queries returned the expected rows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect overwrite conflicts for index commits [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #7972:
URL: https://github.com/apache/paimon/pull/7972#discussion_r3377775235
##########
paimon-core/src/main/java/org/apache/paimon/globalindex/btree/BTreeGlobalIndexBuilder.java:
##########
@@ -133,6 +134,10 @@ public BTreeGlobalIndexBuilder withSnapshot(Snapshot snapshot) {
return this;
}
+ public Optional<Long> scanSnapshotId() {
Review Comment:
Good catch. This method was a leftover from an earlier approach where the BTree index builder exposed its scan snapshot to the commit conflict checker. The current implementation no longer consumes this stateful getter; index commits are validated by checking added global-index row ranges against current data-file row ranges during commit. I removed the unused field/getter and the related assignments in 9c2752005.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect overwrite conflicts for index commits [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 closed pull request #7972: [core][flink] Detect overwrite conflicts for index commits
URL: https://github.com/apache/paimon/pull/7972
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect overwrite conflicts for index commits [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #7972:
URL: https://github.com/apache/paimon/pull/7972
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect overwrite conflicts for index commits [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 closed pull request #7972: [core][flink] Detect overwrite conflicts for index commits
URL: https://github.com/apache/paimon/pull/7972
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect row-id reassign conflicts for index commits [paimon]
Posted by "jerry-024 (via GitHub)" <gi...@apache.org>.
jerry-024 commented on code in PR #7972:
URL: https://github.com/apache/paimon/pull/7972#discussion_r3302591102
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/globalindex/GenericIndexTopoBuilder.java:
##########
@@ -179,6 +179,10 @@ public static boolean buildIndex(
List<ManifestEntry> entries = indexBuilder.scan();
List<IndexManifestEntry> deletedIndexEntries = indexBuilder.deletedIndexEntries();
+ Long rowIdReassignCheckFromSnapshot =
+ indexBuilder.scanSnapshotId().isPresent()
+ ? indexBuilder.scanSnapshotId().get()
+ : null;
Review Comment:
nit: Can be simplified to:
```java
Long rowIdReassignCheckFromSnapshot = indexBuilder.scanSnapshotId().orElse(null);
```
`Optional.orElse(null)` is idiomatic and avoids the double method call.
##########
paimon-core/src/test/java/org/apache/paimon/operation/FileStoreCommitTest.java:
##########
@@ -1008,6 +1009,100 @@ public void testCommitManifestWithProperties() throws Exception {
}
}
+ @Test
+ public void testReplaceManifestListWithRowIdReassignProperty() throws Exception {
+ TestFileStore store = createStore(false);
+
+ List<KeyValue> keyValues = generateDataList(1);
+ BinaryRow partition = gen.getPartition(keyValues.get(0));
+ Snapshot latest = store.commitData(keyValues, s -> partition, kv -> 0).get(0);
+
+ Map<String, String> reassignProperties = new HashMap<>();
+ reassignProperties.put("keep", "v1");
+ reassignProperties.put(Snapshot.ROW_ID_REASSIGN_PROPERTY, "true");
+ try (FileStoreCommitImpl commit = store.newCommit()) {
+ assertThat(
+ commit.replaceManifestList(
+ latest,
+ latest.totalRecordCount(),
+ baseManifestList(latest),
+ deltaManifestList(latest),
+ latest.indexManifest(),
+ latest.nextRowId(),
+ reassignProperties))
+ .isTrue();
+ }
+
+ Snapshot reassignSnapshot = checkNotNull(store.snapshotManager().latestSnapshot());
+ assertThat(reassignSnapshot.properties()).isEqualTo(reassignProperties);
+
+ try (FileStoreCommitImpl commit = store.newCommit()) {
+ assertThat(
+ commit.replaceManifestList(
+ reassignSnapshot,
+ reassignSnapshot.totalRecordCount(),
+ baseManifestList(reassignSnapshot),
+ deltaManifestList(reassignSnapshot),
+ reassignSnapshot.indexManifest(),
+ reassignSnapshot.nextRowId()))
+ .isTrue();
+ }
+
+ Snapshot normalSnapshot = checkNotNull(store.snapshotManager().latestSnapshot());
+ assertThat(normalSnapshot.properties())
+ .containsEntry("keep", "v1")
+ .doesNotContainKey(Snapshot.ROW_ID_REASSIGN_PROPERTY);
+ }
+
+ @Test
+ public void testRowIdReassignConflictFromOptions() throws Exception {
Review Comment:
suggestion: Consider adding a test that verifies `compactManifestOnce` also strips `ROW_ID_REASSIGN_PROPERTY`. Current tests cover `replaceManifestList` but not the compact path.
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/globalindex/GenericIndexTopoBuilder.java:
##########
@@ -506,22 +512,39 @@ private static List<Committable> createDeleteCommittables(
}
private static void commit(
- FileStoreTable table, String indexType, DataStream<Committable> written) {
+ FileStoreTable table,
+ String indexType,
+ DataStream<Committable> written,
+ Long rowIdReassignCheckFromSnapshot) {
+ FileStoreTable commitTable = withRowIdReassignCheck(table, rowIdReassignCheckFromSnapshot);
OneInputStreamOperatorFactory<Committable, Committable> committerOperator =
new CommitterOperatorFactory<>(
false,
true,
"GenericIndexCommitter-" + indexType + "-" + UUID.randomUUID(),
context ->
new StoreCommitter(
- table, table.newCommit(context.commitUser()), context),
+ commitTable,
+ commitTable.newCommit(context.commitUser()),
+ context),
new NoopCommittableStateManager());
written.transform("COMMIT OPERATOR", new CommittableTypeInfo(), committerOperator)
.setParallelism(1)
.setMaxParallelism(1);
}
+ private static FileStoreTable withRowIdReassignCheck(
+ FileStoreTable table, Long rowIdReassignCheckFromSnapshot) {
+ if (rowIdReassignCheckFromSnapshot == null) {
+ return table;
+ }
+ return table.copy(
+ Collections.singletonMap(
+ CoreOptions.COMMIT_ROW_ID_REASSIGN_LAST_SAFE_SNAPSHOT.key(),
+ String.valueOf(rowIdReassignCheckFromSnapshot)));
+ }
+
Review Comment:
nit: This method is identical to `BTreeIndexTopoBuilder.withRowIdReassignCheck`. Consider extracting to a shared utility to avoid drift if the option key or logic changes later. Not blocking — the method is small.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect row-id reassign conflicts for index commits [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #7972:
URL: https://github.com/apache/paimon/pull/7972#discussion_r3302753397
##########
paimon-core/src/main/java/org/apache/paimon/operation/commit/ConflictDetection.java:
##########
@@ -131,6 +132,14 @@ public boolean hasRowIdCheckFromSnapshot() {
return rowIdCheckFromSnapshot != null;
}
+ public void setRowIdReassignCheckFromSnapshot(@Nullable Long rowIdReassignCheckFromSnapshot) {
Review Comment:
Thanks for the suggestion. I think these two snapshot markers should stay separate because they guard different invariants. `rowIdCheckFromSnapshot` is used by row-id range / column-level conflict checks, while this one is driven by `commit.row-id-reassign.last-safe-snapshot` and scans later OVERWRITE snapshots for row-id reassign or overlapped partition overwrite conflicts during index build/compact commits. Reusing the existing field would couple two independent checks and make the COMPACT/index path harder to reason about.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect row-id reassign conflicts for index commits [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #7972:
URL: https://github.com/apache/paimon/pull/7972#discussion_r3302642881
##########
paimon-core/src/main/java/org/apache/paimon/operation/commit/ConflictDetection.java:
##########
@@ -131,6 +132,14 @@ public boolean hasRowIdCheckFromSnapshot() {
return rowIdCheckFromSnapshot != null;
}
+ public void setRowIdReassignCheckFromSnapshot(@Nullable Long rowIdReassignCheckFromSnapshot) {
Review Comment:
That is a different function.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect row-id reassign conflicts for index commits [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #7972:
URL: https://github.com/apache/paimon/pull/7972#discussion_r3302543921
##########
paimon-core/src/main/java/org/apache/paimon/operation/commit/ConflictDetection.java:
##########
@@ -131,6 +132,14 @@ public boolean hasRowIdCheckFromSnapshot() {
return rowIdCheckFromSnapshot != null;
}
+ public void setRowIdReassignCheckFromSnapshot(@Nullable Long rowIdReassignCheckFromSnapshot) {
Review Comment:
Can we just reuse setRowIdCheckFromSnapshot?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect row-id reassign conflicts for index commits [paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #7972:
URL: https://github.com/apache/paimon/pull/7972#discussion_r3302642881
##########
paimon-core/src/main/java/org/apache/paimon/operation/commit/ConflictDetection.java:
##########
@@ -131,6 +132,14 @@ public boolean hasRowIdCheckFromSnapshot() {
return rowIdCheckFromSnapshot != null;
}
+ public void setRowIdReassignCheckFromSnapshot(@Nullable Long rowIdReassignCheckFromSnapshot) {
Review Comment:
That is a different function.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect overwrite conflicts for index commits [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #7972:
URL: https://github.com/apache/paimon/pull/7972#discussion_r3372945096
##########
paimon-core/src/main/java/org/apache/paimon/globalindex/btree/BTreeGlobalIndexBuilder.java:
##########
@@ -133,6 +134,10 @@ public BTreeGlobalIndexBuilder withSnapshot(Snapshot snapshot) {
return this;
}
+ public Optional<Long> scanSnapshotId() {
Review Comment:
Why providing this method?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect overwrite conflicts for index commits [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #7972:
URL: https://github.com/apache/paimon/pull/7972#issuecomment-4656551593
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][flink] Detect overwrite conflicts for index commits [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #7972:
URL: https://github.com/apache/paimon/pull/7972#issuecomment-4593348216
Reviewed commit 36f6f1a. The overwrite-conflict check looks scoped to index commits via the scan snapshot from the index builders, the overwrite barrier is treated as a one-shot marker and stripped from derived snapshots, and the added tests cover barrier, partition-overlap, non-overwrite, and data-only cases. I didn't find blocking issues from my side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org