XiaoHongbo-Hope commented on code in PR #8358:
URL: https://github.com/apache/paimon/pull/8358#discussion_r3475348172
##########
paimon-python/pypaimon/write/commit/commit_scanner.py:
##########
@@ -116,6 +116,26 @@ def
read_incremental_raw_entries_from_changed_partitions(self, snapshot: Snapsho
entries.append(entry)
return entries
+ def read_incremental_changes(self, from_snapshot: Snapshot, to_snapshot:
Snapshot,
+ commit_entries: List[ManifestEntry]):
+ """Read delta entries (including DELETEs) for snapshots in the range
+ ``(from_snapshot, to_snapshot]``, filtered to the changed partitions.
+
+ Lets a commit retry reuse the base entries computed by the previous
+ attempt and only read what concurrent writers committed since, instead
+ of re-scanning the whole changed partitions. Mirrors Java
+ ``CommitScanner#readIncrementalChanges``.
+ """
+ snapshot_manager = self.table.snapshot_manager()
+ entries = []
+ for snapshot_id in range(from_snapshot.id + 1, to_snapshot.id + 1):
+ snapshot = snapshot_manager.get_snapshot_by_id(snapshot_id)
+ if snapshot is None:
Review Comment:
> This is slightly weaker than the Java implementation. Java
`CommitScanner#readIncrementalChanges` scans every snapshot id in `(from, to]`
via `scan.withSnapshot(i)`, so a missing/expired intermediate snapshot is not
treated as an empty delta. Here, skipping `None` while reusing
`base_data_files` can make the reconstructed base incomplete and allow conflict
detection to run on a state that is not equivalent to `latest_snapshot`. Could
we either fall back to `read_all_entries_from_changed_partitions(to_snapshot,
commit_entries)` when any intermediate snapshot is missing, or fail
conservatively so the retry does a full scan?
Fixed
--
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]
