hameizi commented on code in PR #10541:
URL: https://github.com/apache/paimon/pull/10541#discussion_r4313998071
##########
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:
Fixed. The latest-snapshot resolver now retains evidence of a non-empty
table across the entire relocation retry loop. A later empty resolution no
longer returns None after such evidence has been observed. It retries within
the existing bound
and raises SnapshotNotResolvable if resolution cannot recover.
During the follow-up checks, I also addressed writer/committer
bucket-configuration inconsistencies. Effective write options are applied
before routing and bucket planning, and table-copy provenance is preserved. On
every initialization or
reconstruction, the actor applies the dynamic options to its reloaded table
and validates the resulting bucket configuration against the writer’s captured
configuration. A mismatch fails the write instead of silently publishing
inconsistent
manifest metadata. This is fail-closed validation, not full schema-version
pinning.
Added regressions for the real-filesystem retention/listing race, branch and
table. copy cases, catalog bucket changes before actor startup, and actual Ray
actor reconstruction. Successful-write controls verify equality-filtered reads
and
manifest bucket counts, not just full-scan row counts. The related
regression suite passes: 213 tests and 29 subtests.
--
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]
