uros-b commented on code in PR #17754:
URL: https://github.com/apache/iceberg/pull/17754#discussion_r3839625285
##########
core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java:
##########
@@ -1183,12 +1289,33 @@ private List<DeleteFile> mergeDVs() {
String.format(
"merged-dvs-%s-%s", snapshotId(),
dvMergeAttempt.incrementAndGet())));
- return DVUtil.mergeAndWriteDVsIfRequired(
- dvsByReferencedFile,
- dvOutputLocation,
- fileIO,
- ops().current().specsById(),
- ThreadPools.getDeleteWorkerPool());
+ List<DeleteFile> mergedDVs =
+ DVUtil.mergeAndWriteDVsIfRequired(
+ dvsByReferencedFile,
+ dvOutputLocation,
+ fileIO,
+ ops().current().specsById(),
+ ThreadPools.getDeleteWorkerPool());
+
+ if (requiresMerge) {
+ mergedDVFileLocations.add(dvOutputLocation);
Review Comment:
The merged-DV cleanup tracks the raw location string,
mergedDVFileLocations.add(dvOutputLocation) (from
locationProvider().newDataLocation(...)), and later deletes it via
deleteFile(dvOutputLocation). The overlapping #17309 deliberately tracks the
FileIO-resolved fileIO.newOutputFile(dvOutputLocation).location() instead (its
comment: the raw location "may differ from … DeleteFile.location() (e.g.
stripping a file: URI scheme)"). Deletion by the raw write-string is symmetric
for the standard FileIO implementations (local/Hadoop/S3 —
testConcurrentDVMergeCleanupOnCommitFailure passes), so this is not a
demonstrated leak; but resolving through the FileIO guarantees cleanup targets
the canonical written location under any FileIO or a user-supplied deleteWith
callback, and aligns with #17309. Recommend adopting the resolved form.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
