This is an automated email from the ASF dual-hosted git repository.
Gabriel39 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new fd807abdd49 [fix](iceberg) Cleanup when drop DB (#64269)
fd807abdd49 is described below
commit fd807abdd4926795ae20a6a526be4511ce96b934
Author: Gabriel <[email protected]>
AuthorDate: Thu Jun 11 09:47:33 2026 +0800
[fix](iceberg) Cleanup when drop DB (#64269)
---
.../datasource/iceberg/IcebergMetadataOps.java | 113 +++++++++-
.../datasource/iceberg/IcebergMetadataOpTest.java | 229 +++++++++++++++++++++
2 files changed, 340 insertions(+), 2 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
index e6612f03680..bb2d1debd8b 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
@@ -44,6 +44,11 @@ import org.apache.doris.datasource.ExternalTable;
import org.apache.doris.datasource.operations.ExternalMetadataOps;
import org.apache.doris.datasource.property.metastore.IcebergRestProperties;
import org.apache.doris.datasource.property.metastore.MetastoreProperties;
+import org.apache.doris.filesystem.FileEntry;
+import org.apache.doris.filesystem.FileIterator;
+import org.apache.doris.filesystem.FileSystem;
+import org.apache.doris.filesystem.Location;
+import org.apache.doris.fs.SpiSwitchingFileSystem;
import org.apache.doris.nereids.trees.plans.commands.info.AddPartitionFieldOp;
import org.apache.doris.nereids.trees.plans.commands.info.CreateTableInfo;
import org.apache.doris.nereids.trees.plans.commands.info.DropPartitionFieldOp;
@@ -52,6 +57,7 @@ import
org.apache.doris.nereids.trees.plans.commands.info.SortFieldInfo;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
+import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.iceberg.ManageSnapshots;
import org.apache.iceberg.PartitionSpec;
@@ -79,6 +85,7 @@ import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.jetbrains.annotations.NotNull;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -93,6 +100,8 @@ import java.util.stream.Stream;
public class IcebergMetadataOps implements ExternalMetadataOps {
private static final Logger LOG =
LogManager.getLogger(IcebergMetadataOps.class);
+ private static final String NAMESPACE_LOCATION_PROP = "location";
+ private static final List<String> ICEBERG_TABLE_LOCATION_CHILD_DIRS =
Arrays.asList("data", "metadata");
protected Catalog catalog;
protected ExternalCatalog dorisCatalog;
protected SupportsNamespaces nsCatalog;
@@ -299,7 +308,11 @@ public class IcebergMetadataOps implements
ExternalMetadataOps {
return;
}
}
- nsCatalog.dropNamespace(getNamespace(dorisDb.getRemoteName()));
+ Namespace namespace = getNamespace(dorisDb.getRemoteName());
+ Optional<String> namespaceLocation = shouldCleanupManagedLocation()
+ ? loadNamespaceLocation(namespace) : Optional.empty();
+ nsCatalog.dropNamespace(namespace);
+ namespaceLocation.ifPresent(location -> cleanupEmptyLocation(location,
"database", dbName, false));
}
@Override
@@ -427,7 +440,103 @@ public class IcebergMetadataOps implements
ExternalMetadataOps {
ErrorReport.reportDdlException(ErrorCode.ERR_UNKNOWN_TABLE,
remoteTblName, remoteDbName);
}
}
- catalog.dropTable(getTableIdentifier(remoteDbName, remoteTblName),
true);
+ TableIdentifier tableIdentifier = getTableIdentifier(remoteDbName,
remoteTblName);
+ Optional<String> tableLocation = shouldCleanupManagedLocation()
+ ? loadTableLocation(tableIdentifier) : Optional.empty();
+ catalog.dropTable(tableIdentifier, true);
+ tableLocation.ifPresent(location ->
+ cleanupEmptyLocation(location, "table", remoteDbName + "." +
remoteTblName, true));
+ }
+
+ private Optional<String> loadNamespaceLocation(Namespace namespace) {
+ Map<String, String> namespaceMetadata =
nsCatalog.loadNamespaceMetadata(namespace);
+ String location = namespaceMetadata.get(NAMESPACE_LOCATION_PROP);
+ return StringUtils.isBlank(location) ? Optional.empty() :
Optional.of(location);
+ }
+
+ private Optional<String> loadTableLocation(TableIdentifier
tableIdentifier) {
+ String location = catalog.loadTable(tableIdentifier).location();
+ return StringUtils.isBlank(location) ? Optional.empty() :
Optional.of(location);
+ }
+
+ private void cleanupEmptyLocation(
+ String location, String objectType, String objectName, boolean
cleanupIcebergTableChildren) {
+ if (!shouldCleanupManagedLocation() || StringUtils.isBlank(location)) {
+ return;
+ }
+ try (FileSystem fs = createCleanupFileSystem()) {
+ boolean deleted = cleanupIcebergTableChildren
+ ? deleteEmptyTableLocation(fs, Location.of(location))
+ : deleteEmptyDirectory(fs, Location.of(location));
+ if (deleted) {
+ LOG.info("Cleaned empty Iceberg {} location {} for {}",
objectType, location, objectName);
+ } else {
+ LOG.info("Skip cleaning Iceberg {} location {} for {}, because
it still contains files",
+ objectType, location, objectName);
+ }
+ } catch (Exception e) {
+ LOG.warn("Failed to clean Iceberg {} location {} for {} after
drop",
+ objectType, location, objectName, e);
+ }
+ }
+
+ private boolean shouldCleanupManagedLocation() {
+ // Only cleanup HMS-Iceberg location
+ return dorisCatalog instanceof IcebergExternalCatalog
+ && IcebergExternalCatalog.ICEBERG_HMS.equals(
+ ((IcebergExternalCatalog)
dorisCatalog).getIcebergCatalogType());
+ }
+
+ @VisibleForTesting
+ protected FileSystem createCleanupFileSystem() {
+ return new
SpiSwitchingFileSystem(dorisCatalog.getCatalogProperty().getStoragePropertiesMap());
+ }
+
+ @VisibleForTesting
+ static boolean deleteEmptyDirectory(FileSystem fs, Location location)
throws IOException {
+ if (!fs.exists(location)) {
+ return true;
+ }
+ List<Location> childDirectories = new ArrayList<>();
+ try (FileIterator iterator = fs.list(location)) {
+ while (iterator.hasNext()) {
+ FileEntry entry = iterator.next();
+ if (!entry.isDirectory()) {
+ return false;
+ }
+ childDirectories.add(entry.location());
+ }
+ }
+ for (Location childDirectory : childDirectories) {
+ if (!deleteEmptyDirectory(fs, childDirectory)) {
+ return false;
+ }
+ }
+ return deleteEmptyDirectoryMarker(fs, location);
+ }
+
+ @VisibleForTesting
+ static boolean deleteEmptyTableLocation(FileSystem fs, Location location)
throws IOException {
+ for (String childDir : ICEBERG_TABLE_LOCATION_CHILD_DIRS) {
+ if (!deleteEmptyDirectory(fs, location.resolve(childDir))) {
+ return false;
+ }
+ }
+ return deleteEmptyDirectory(fs, location);
+ }
+
+ private static boolean deleteEmptyDirectoryMarker(FileSystem fs, Location
location) throws IOException {
+ Location directoryMarker =
Location.of(withTrailingSlash(location.uri()));
+ try {
+ fs.delete(directoryMarker, false);
+ } catch (IOException e) {
+ return !fs.exists(location);
+ }
+ return !fs.exists(location);
+ }
+
+ private static String withTrailingSlash(String uri) {
+ return uri.endsWith("/") ? uri : uri + "/";
}
public void renameTableImpl(String dbName, String tblName, String
newTblName) throws DdlException {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpTest.java
index 2bf1ef3deb4..079a0c9c312 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpTest.java
@@ -19,6 +19,15 @@ package org.apache.doris.datasource.iceberg;
import org.apache.doris.common.security.authentication.ExecutionAuthenticator;
import org.apache.doris.datasource.CatalogProperty;
+import org.apache.doris.datasource.ExternalDatabase;
+import org.apache.doris.datasource.ExternalTable;
+import org.apache.doris.filesystem.DorisInputFile;
+import org.apache.doris.filesystem.DorisOutputFile;
+import org.apache.doris.filesystem.FileEntry;
+import org.apache.doris.filesystem.FileIterator;
+import org.apache.doris.filesystem.FileSystem;
+import org.apache.doris.filesystem.Location;
+import org.apache.doris.fs.MemoryFileSystem;
import org.apache.iceberg.catalog.Catalog;
import org.apache.iceberg.catalog.Namespace;
@@ -29,12 +38,17 @@ import org.junit.Assert;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
+import java.io.IOException;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.NoSuchElementException;
import java.util.Optional;
+import java.util.Set;
public class IcebergMetadataOpTest {
@@ -114,4 +128,219 @@ public class IcebergMetadataOpTest {
Assert.assertEquals(Collections.singletonList("DORIS_HORIZON_T"),
tableNames);
}
+
+ @Test
+ public void testDropTableCleansEmptyTableLocation() throws Exception {
+ MemoryFileSystem fs = new MemoryFileSystem();
+ Location tableLocation = Location.of("hdfs://nn/warehouse/db/t1");
+ fs.mkdirs(tableLocation);
+ fs.mkdirs(tableLocation.resolve("data"));
+ fs.mkdirs(tableLocation.resolve("metadata"));
+
+ IcebergExternalCatalog dorisCatalog = mockHmsCatalog();
+ Catalog icebergCatalog = Mockito.mock(Catalog.class,
+
Mockito.withSettings().extraInterfaces(SupportsNamespaces.class));
+ IcebergMetadataOps ops = newOpsWithCleanupFileSystem(dorisCatalog,
icebergCatalog, fs);
+
+ TableIdentifier tableIdentifier =
TableIdentifier.of(Namespace.of("db"), "t1");
+ org.apache.iceberg.Table icebergTable =
Mockito.mock(org.apache.iceberg.Table.class);
+ Mockito.when(icebergTable.location()).thenReturn(tableLocation.uri());
+
Mockito.when(icebergCatalog.tableExists(tableIdentifier)).thenReturn(true);
+
Mockito.when(icebergCatalog.loadTable(tableIdentifier)).thenReturn(icebergTable);
+ Mockito.when(icebergCatalog.dropTable(tableIdentifier,
true)).thenReturn(true);
+
+ ExternalTable dorisTable = Mockito.mock(ExternalTable.class);
+ Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db");
+ Mockito.when(dorisTable.getRemoteName()).thenReturn("t1");
+ Mockito.when(dorisTable.getName()).thenReturn("t1");
+ ops.dropTableImpl(dorisTable, false);
+
+ Assert.assertFalse(fs.exists(tableLocation));
+ Mockito.verify(icebergCatalog).dropTable(tableIdentifier, true);
+ }
+
+ @Test
+ public void testDropDbCleansEmptyNamespaceLocation() throws Exception {
+ MemoryFileSystem fs = new MemoryFileSystem();
+ Location namespaceLocation = Location.of("hdfs://nn/warehouse/db.db");
+ fs.mkdirs(namespaceLocation);
+
+ IcebergExternalCatalog dorisCatalog = mockHmsCatalog();
+ Catalog icebergCatalog = Mockito.mock(Catalog.class,
+
Mockito.withSettings().extraInterfaces(SupportsNamespaces.class));
+ IcebergMetadataOps ops = newOpsWithCleanupFileSystem(dorisCatalog,
icebergCatalog, fs);
+
+ ExternalDatabase<?> dorisDb = Mockito.mock(ExternalDatabase.class);
+ Mockito.when(dorisDb.getRemoteName()).thenReturn("db");
+ Mockito.doReturn(dorisDb).when(dorisCatalog).getDbNullable("db");
+
+ SupportsNamespaces nsCatalog = (SupportsNamespaces) icebergCatalog;
+ Namespace namespace = Namespace.of("db");
+ Mockito.when(nsCatalog.loadNamespaceMetadata(namespace))
+ .thenReturn(Collections.singletonMap("location",
namespaceLocation.uri()));
+ Mockito.when(nsCatalog.dropNamespace(namespace)).thenReturn(true);
+ ops.dropDbImpl("db", false, false);
+
+ Assert.assertFalse(fs.exists(namespaceLocation));
+ Mockito.verify(nsCatalog).dropNamespace(namespace);
+ }
+
+ @Test
+ public void testDeleteEmptyDirectoryKeepsDirectoryWithExternalFile()
throws Exception {
+ MemoryFileSystem fs = new MemoryFileSystem();
+ Location tableLocation = Location.of("hdfs://nn/warehouse/db/t2");
+ fs.mkdirs(tableLocation);
+ fs.mkdirs(tableLocation.resolve("data"));
+ Location externalFile = tableLocation.resolve("external-file");
+ fs.put(externalFile, new byte[] {1});
+
+ Assert.assertFalse(IcebergMetadataOps.deleteEmptyDirectory(fs,
tableLocation));
+ Assert.assertTrue(fs.exists(tableLocation));
+ Assert.assertTrue(fs.exists(externalFile));
+ Assert.assertTrue(fs.exists(tableLocation.resolve("data")));
+ }
+
+ @Test
+ public void testDeleteEmptyTableLocationCleansFlatObjectStoreMarkers()
throws Exception {
+ FlatMarkerFileSystem fs = new FlatMarkerFileSystem();
+ Location tableLocation = Location.of("s3://bucket/warehouse/db/t3");
+ fs.mkdirs(tableLocation);
+ fs.mkdirs(tableLocation.resolve("data"));
+ fs.mkdirs(tableLocation.resolve("metadata"));
+
+ Assert.assertTrue(fs.exists(tableLocation));
+ Assert.assertTrue(IcebergMetadataOps.deleteEmptyTableLocation(fs,
tableLocation));
+ Assert.assertFalse(fs.exists(tableLocation));
+ }
+
+ private IcebergExternalCatalog mockHmsCatalog() {
+ IcebergExternalCatalog dorisCatalog =
Mockito.mock(IcebergExternalCatalog.class);
+ Mockito.when(dorisCatalog.getExecutionAuthenticator()).thenReturn(new
ExecutionAuthenticator() {
+ });
+
Mockito.when(dorisCatalog.getProperties()).thenReturn(Collections.emptyMap());
+
Mockito.when(dorisCatalog.getIcebergCatalogType()).thenReturn(IcebergExternalCatalog.ICEBERG_HMS);
+ Mockito.when(dorisCatalog.getCatalogProperty()).thenReturn(new
CatalogProperty(null, Collections.emptyMap()));
+ return dorisCatalog;
+ }
+
+ private IcebergMetadataOps newOpsWithCleanupFileSystem(
+ IcebergExternalCatalog dorisCatalog, Catalog icebergCatalog,
FileSystem fs) {
+ IcebergMetadataOps ops = new IcebergMetadataOps(dorisCatalog,
icebergCatalog) {
+ @Override
+ protected FileSystem createCleanupFileSystem() {
+ return fs;
+ }
+ };
+ Mockito.when(dorisCatalog.getMetadataOps()).thenReturn(ops);
+ return ops;
+ }
+
+ private static class FlatMarkerFileSystem implements FileSystem {
+ private final Set<String> markers = new HashSet<>();
+ private final Set<String> files = new HashSet<>();
+
+ @Override
+ public boolean exists(Location location) {
+ String uri = location.uri();
+ String marker = withTrailingSlash(uri);
+ if (markers.contains(uri) || markers.contains(marker) ||
files.contains(uri)) {
+ return true;
+ }
+ String prefix = withTrailingSlash(uri);
+ for (String file : files) {
+ if (file.startsWith(prefix)) {
+ return true;
+ }
+ }
+ for (String directoryMarker : markers) {
+ if (directoryMarker.startsWith(prefix)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public void mkdirs(Location location) {
+ markers.add(withTrailingSlash(location.uri()));
+ }
+
+ @Override
+ public void delete(Location location, boolean recursive) throws
IOException {
+ if (recursive) {
+ throw new IOException("recursive delete is not fail-safe");
+ }
+ String marker = withTrailingSlash(location.uri());
+ for (String file : files) {
+ if (file.startsWith(marker) && !file.equals(marker)) {
+ throw new IOException("Directory not empty: " +
location.uri());
+ }
+ }
+ for (String directoryMarker : markers) {
+ if (directoryMarker.startsWith(marker) &&
!directoryMarker.equals(marker)) {
+ throw new IOException("Directory not empty: " +
location.uri());
+ }
+ }
+ markers.remove(marker);
+ files.remove(location.uri());
+ }
+
+ @Override
+ public void rename(Location src, Location dst) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public FileIterator list(Location location) {
+ String prefix = withTrailingSlash(location.uri());
+ List<FileEntry> entries = new ArrayList<>();
+ for (String file : files) {
+ if (file.startsWith(prefix)) {
+ entries.add(new FileEntry(Location.of(file), 1L, false,
0L, null));
+ }
+ }
+ return iteratorOf(entries);
+ }
+
+ @Override
+ public DorisInputFile newInputFile(Location location) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public DorisOutputFile newOutputFile(Location location) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void close() {
+ }
+
+ private static FileIterator iteratorOf(List<FileEntry> entries) {
+ return new FileIterator() {
+ private int index = 0;
+
+ @Override
+ public boolean hasNext() {
+ return index < entries.size();
+ }
+
+ @Override
+ public FileEntry next() {
+ if (!hasNext()) {
+ throw new NoSuchElementException();
+ }
+ return entries.get(index++);
+ }
+
+ @Override
+ public void close() {
+ }
+ };
+ }
+
+ private static String withTrailingSlash(String uri) {
+ return uri.endsWith("/") ? uri : uri + "/";
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
