Guosmilesmile commented on code in PR #18674:
URL: https://github.com/apache/iceberg/pull/18674#discussion_r4421999201
##########
flink/v2.3/flink/src/main/java/org/apache/iceberg/flink/maintenance/operator/EqualityConvertPlanner.java:
##########
@@ -350,20 +341,58 @@ private void ensureIndexCurrent(
rebuildIndex(mainSnapshot, !bootstrap);
}
+ indexSnapshotId = mainSnapshot.snapshotId();
+ pendingStagingSnapshotId = null;
lastMainSnapshotId = mainSnapshot.snapshotId();
}
+ /**
+ * Returns whether the index reflects {@code mainSnapshot}.
+ *
+ * <p>Planning a staging snapshot changes the index before its conversion
commit exists: its eq
+ * deletes consume the entries they match and its data files are added. So
with a snapshot
+ * planned, the index reflects the head only if that snapshot's conversion
commit sits directly on
+ * top of the indexed snapshot. Without one, the head must be the indexed
snapshot. Any other head
+ * holds commits the index never saw (external commits, rollbacks, a restore
from before a
+ * conversion commit) or follows a cycle that did not commit.
+ *
+ * <p>After a restore, the conversion commit can come from a retry of the
planned cycle. A cycle
+ * that only failed to commit leaves the same index as its retry. A rebuild
that fails part way
+ * clears {@link #indexSnapshotId}, and the committer does not commit a
conversion on a generation
+ * whose update failed downstream.
+ */
+ private boolean indexReflects(Snapshot mainSnapshot) {
Review Comment:
One scenario I'd like to confirm before merging , is a job that already has
a corrupted index guaranteed to rebuild on the first trigger after the upgrade?
(1) an external commit E lands on the target, the rebuild fails in a reader,
so the worker index is missing one file's rows. The old planner set
indexSnapshotId = E before emitting, so the savepoint records indexSnapshotId =
E. (2) The job then plans S2 and commits it on top of E, writing marker S2 with
parent E — state is now indexSnapshotId = E, pending = S2.
(3) Savepoint, deploy, restore: head C2 has marker S2 and parent E, so
indexReflects() returns true and no rebuild happens, even though the missing
rows belong to E.
The same holds when nothing was planned yet (pending == null), where head ==
indexSnapshotId is enough to reuse. failedIndexGeneration can't help either,
since that state doesn't exist in an old savepoint.
Am I missing something here?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
