JingsongLi commented on code in PR #7715:
URL: https://github.com/apache/paimon/pull/7715#discussion_r3443113772
##########
paimon-core/src/main/java/org/apache/paimon/operation/LocalOrphanFilesClean.java:
##########
@@ -114,6 +114,15 @@ public CleanOrphanFilesResult clean()
.flatMap(branch -> getUsedFiles(branch).stream())
.collect(Collectors.toSet());
+ if (usedFiles.isEmpty()) {
Review Comment:
This guard still does not protect the race described in the PR.
`collectWithoutDataFileWithManifestFlag` adds the snapshot manifest-list names
to `usedFiles` before reading the manifest-list contents, and
`retryReadingFiles(..., emptyList())` returns an empty list when those
manifest-lists are concurrently removed. In that case `usedFiles` is not empty,
so this check is bypassed, but none of the data files referenced by the expired
manifests are protected and `candidateDeletes.removeAll(usedFiles)` can still
delete them. Can we guard the data-file collection result instead, or treat a
missing manifest-list as an abort for the branch/table rather than only
checking whether the overall used-file set 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]
