Skip to content

Commit 7be0710

Browse files
Aireedxxubai
andauthored
[AMORO-4277] imporove expire_snapshots performance (#4278)
* optimize expire snapshots fix: logical error remove some comments * remove duplicate class and move test case to amoro-format-iceberg * Update amoro-format-iceberg/src/main/java/org/apache/amoro/utils/TableFileUtil.java Co-authored-by: Xu Bai <tocreationbai@gmail.com> * fix review comment --------- Co-authored-by: Xu Bai <tocreationbai@gmail.com>
1 parent eee2e35 commit 7be0710

4 files changed

Lines changed: 44 additions & 156 deletions

File tree

amoro-ams/src/main/java/org/apache/amoro/server/utils/RollingFileCleaner.java

Lines changed: 0 additions & 138 deletions
This file was deleted.

amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/utils/RollingFileCleaner.java

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@
2727
import org.slf4j.LoggerFactory;
2828

2929
import java.net.URI;
30+
import java.util.Comparator;
31+
import java.util.List;
3032
import java.util.Set;
3133
import java.util.concurrent.atomic.AtomicInteger;
34+
import java.util.stream.Collectors;
3235

3336
/** Rolling file cleaner for Iceberg table maintenance operations. */
3437
public class RollingFileCleaner {
@@ -110,27 +113,34 @@ private void doCleanFiles() {
110113
}
111114
}
112115
}
113-
// Try to delete empty parent directories. Skipped automatically by
114-
// TableFileUtil.deleteEmptyDirectory when the underlying FileIO is an object store.
115-
if (fileIO.supportFileSystemOperations()) {
116-
for (String parentDir : parentDirectories) {
117-
try {
118-
TableFileUtil.deleteEmptyDirectory(fileIO, parentDir, excludeFiles);
119-
} catch (Exception e) {
120-
LOG.warn("Failed to delete empty parent directory: {}", parentDir, e);
121-
}
122-
}
123-
}
124116

125117
LOG.debug("Cleaned expired a file group, total files: {}", collectedFiles.size());
126118
} finally {
127-
// Always drop both buffers so a failure in directory cleanup does not cause the
128-
// already-deleted files to be re-submitted on the next batch.
129-
parentDirectories.clear();
130119
collectedFiles.clear();
131120
}
132121
}
133122

123+
private void doCleanParentDirectory() {
124+
// Try to delete empty parent directories. Skipped automatically by
125+
// TableFileUtil.deleteEmptyDirectory when the underlying FileIO is an object store.
126+
if (fileIO.supportFileSystemOperations()) {
127+
List<String> parentDirectoriesSorted =
128+
parentDirectories.stream()
129+
.sorted(Comparator.comparingInt(String::length).reversed())
130+
.collect(Collectors.toList());
131+
for (String parentDir : parentDirectoriesSorted) {
132+
try {
133+
TableFileUtil.deleteEmptyDirectory(fileIO, parentDir, excludeFiles, parentDirectories);
134+
} catch (Exception e) {
135+
LOG.warn("Failed to delete empty parent directory: {}", parentDir, e);
136+
}
137+
}
138+
}
139+
// Always drop both buffers so a failure in directory cleanup does not cause the
140+
// already-deleted files to be re-submitted on the next batch.
141+
parentDirectories.clear();
142+
}
143+
134144
public int fileCount() {
135145
return fileCounter.get();
136146
}
@@ -143,6 +153,9 @@ public void clear() {
143153
if (!collectedFiles.isEmpty()) {
144154
doCleanFiles();
145155
}
156+
if (!parentDirectories.isEmpty()) {
157+
doCleanParentDirectory();
158+
}
146159

147160
collectedFiles.clear();
148161
excludeFiles.clear();

amoro-format-iceberg/src/main/java/org/apache/amoro/utils/TableFileUtil.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
import java.io.File;
3030
import java.net.URI;
31+
import java.util.HashSet;
3132
import java.util.Set;
3233
import java.util.concurrent.ExecutorService;
3334
import java.util.concurrent.atomic.AtomicInteger;
@@ -75,15 +76,25 @@ public static String getFileDir(String filePath) {
7576
return filePath.substring(0, lastSlash);
7677
}
7778

79+
public static void deleteEmptyDirectory(
80+
AuthenticatedFileIO io, String directoryPath, Set<String> exclude) {
81+
deleteEmptyDirectory(io, directoryPath, exclude, new HashSet<>());
82+
}
83+
7884
/**
7985
* Try to recursiveDelete the empty directory
8086
*
8187
* @param io mixed-format file io
8288
* @param directoryPath directory location
8389
* @param exclude the directory will not be deleted
90+
* @param directoriesToBeDeleted: all the directories that need to be deleted directories that
91+
* need to be deleted
8492
*/
8593
public static void deleteEmptyDirectory(
86-
AuthenticatedFileIO io, String directoryPath, Set<String> exclude) {
94+
AuthenticatedFileIO io,
95+
String directoryPath,
96+
Set<String> exclude,
97+
Set<String> directoriesToBeDeleted) {
8798
if (directoryPath == null || directoryPath.isEmpty()) {
8899
return;
89100
}
@@ -108,7 +119,10 @@ public static void deleteEmptyDirectory(
108119
if (io.asFileSystemIO().isEmptyDirectory(directoryPath)) {
109120
io.asFileSystemIO().deletePrefix(directoryPath);
110121
LOG.debug("success delete empty directory {}", directoryPath);
111-
deleteEmptyDirectory(io, parent, exclude);
122+
// for parent must be deleted after the sub-directory
123+
if (directoriesToBeDeleted == null || !directoriesToBeDeleted.contains(parent)) {
124+
deleteEmptyDirectory(io, parent, exclude);
125+
}
112126
}
113127
}
114128

amoro-ams/src/test/java/org/apache/amoro/server/util/TestRollingFileCleaner.java renamed to amoro-format-iceberg/src/test/java/org/apache/amoro/formats/iceberg/utils/TestRollingFileCleaner.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,10 @@
1616
* limitations under the License.
1717
*/
1818

19-
package org.apache.amoro.server.util;
19+
package org.apache.amoro.formats.iceberg.utils;
2020

2121
import org.apache.amoro.io.AuthenticatedFileIO;
2222
import org.apache.amoro.io.AuthenticatedFileIOAdapter;
23-
import org.apache.amoro.server.utils.RollingFileCleaner;
2423
import org.apache.amoro.shade.guava32.com.google.common.collect.Sets;
2524
import org.apache.iceberg.inmemory.InMemoryFileIO;
2625
import org.junit.jupiter.api.Assertions;

0 commit comments

Comments
 (0)