JingsongLi commented on code in PR #10582:
URL: https://github.com/apache/paimon/pull/10582#discussion_r4281711640
##########
paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java:
##########
@@ -725,7 +726,14 @@ public void abort(List<CommitMessage> commitMessages) {
List<DataFileMeta> dataFilesToDelete = new ArrayList<>();
dataFilesToDelete.addAll(commitMessage.newFilesIncrement().newFiles());
dataFilesToDelete.addAll(commitMessage.newFilesIncrement().changelogFiles());
-
dataFilesToDelete.addAll(commitMessage.compactIncrement().compactAfter());
+ // Level upgrades reuse files which are still referenced by the
previous snapshot.
+ Set<Path> compactInputs =
+ commitMessage.compactIncrement().compactBefore().stream()
+ .map(dataPathFactory::toPath)
+ .collect(Collectors.toSet());
+ commitMessage.compactIncrement().compactAfter().stream()
+ .filter(file ->
!compactInputs.contains(dataPathFactory.toPath(file)))
Review Comment:
[P2] Preserve reused paths while cleaning newly created sidecars
Matching the main data-file path does not mean that all of
compactAfter.extraFiles are reused. A lookup level upgrade can retain its main
file while LookupMergeTreeCompactRewriter.notifyRewriteCompactAfter() calls
RemoteLookupFileManager.genRemoteLookupFile() and appends a newly created
remote SST. File-index rebuilds also emit CompactIncrement(before=[old
metadata], after=[same main file with a new .index sidecar]). This filter drops
the entire after metadata from abort cleanup, leaving those uncommitted
sidecars behind. The remote lookup manager has no later abort cleanup for them.
I extended the new regression case with after =
reused.upgrade(1).copy(oldExtras + newSst), preserving the existing
main/extra-file assertions and requiring the newly written SST to be removed.
Both internal and external cases fail on this head because the new SST still
exists. An isolated change that protects every before.collectFiles() path and
deletes only after.collectFiles() paths outside that set makes both cases pass.
Please clean up by physical-path difference rather than skipping the entire
DataFileMeta.
--
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]
