JingsongLi commented on code in PR #10541:
URL: https://github.com/apache/paimon/pull/10541#discussion_r4282956862
##########
paimon-python/pypaimon/snapshot/snapshot_manager.py:
##########
@@ -98,18 +114,67 @@ def _get_latest_snapshot_from_filesystem(self) ->
Optional[Snapshot]:
hint: when it is missing, stale, or unreadable, reconcile with a scan
of the snapshot files, like the Java SnapshotManager.findLatest.
+ Resolving an id and reading that id's file are two steps, and the
+ snapshot resolved can be expired between them -- another writer commits
+ past it and retention drops it. That is a recoverable race, not a
+ failure: the snapshot that replaced it is readable, so the candidate is
+ re-resolved and re-read rather than letting the vanished file reach the
+ caller. Handling it here keeps every caller of this read covered,
+ rather than each one catching the race separately.
+
If the hint indicates the table has snapshots but none can be resolved
(e.g. an unreadable LATEST plus a listing that an IO error truncated to
- empty), fail closed rather than report the table as empty -- callers
- such as the immutable-option guard must not mistake a populated table
- for an empty one.
+ empty), fail closed with :class:`SnapshotNotResolvable` rather than
+ report the table as empty -- callers such as the immutable-option guard
+ must not mistake a populated table for an empty one.
Returns:
The latest snapshot, or None if the table genuinely has none
"""
if not self.file_io.exists(self.snapshot_dir):
return None
+ for attempt in range(LATEST_SNAPSHOT_RELOCATE_ATTEMPTS + 1):
+ latest_snapshot_id, hint_indicates_data, hint_read_error = (
+ self._resolve_latest_snapshot_id())
+
+ if latest_snapshot_id is None:
+ if hint_indicates_data:
+ raise SnapshotNotResolvable(
+ f"Snapshot hint under {self.snapshot_dir} indicates
existing "
+ f"snapshots but none could be resolved"
+ + (f": {hint_read_error}" if hint_read_error else ""))
+ return None
Review Comment:
[P2] Retain non-empty evidence across relocation attempts
With a missing best-effort LATEST hint, a later resolution returning no ID
does not mean this call has established an empty table. I reproduced this with
real LocalFileIO and ordinary batch commits: resolution finds snapshot 1;
before its read, commit 2 replaces it and retention removes 1. During the
retry, directory iteration captures snapshot 2, then commit 3 replaces it
before stat; LocalFileIO skips the vanished entry and returns an empty listing.
This branch returns None, so a normal table read silently returns [], although
snapshot 3 is readable and contains [1, 2, 3]. The exact baseline raises
FileNotFoundError on the first race. No rollback/reset or mocked empty listing
is involved. Preserve the positive evidence from earlier attempts and retry or
raise SnapshotNotResolvable when a later resolution is empty.
--
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]
