This is an automated email from the ASF dual-hosted git repository.
mchades pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 2631d87c96 [Cherry-pick to branch-1.3] [#9954] fix(lakehouse-generic):
Enhance purgeTable logic to handle external tables and missing locations.
(#9967) (#11463)
2631d87c96 is described below
commit 2631d87c968f641568d54a1310e79aa5e6d4b13e
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Jun 5 23:38:22 2026 +0800
[Cherry-pick to branch-1.3] [#9954] fix(lakehouse-generic): Enhance
purgeTable logic to handle external tables and missing locations. (#9967)
(#11463)
**Cherry-pick Information:**
- Original commit: 1d365c7cd6c48844dcf7c5ceac2f5c794aab2124
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Qi Yu <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
---
.../lakehouse/lance/LanceTableOperations.java | 58 +++++++++++-----
.../test/CatalogGenericCatalogLanceIT.java | 79 ++++++++++++++++++++++
.../lance/integration/test/LanceRESTServiceIT.java | 51 +++++++++++++-
3 files changed, 171 insertions(+), 17 deletions(-)
diff --git
a/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
b/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
index 9edec20a23..89c99d5365 100644
---
a/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
+++
b/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
@@ -23,6 +23,7 @@ import static
org.apache.gravitino.lance.common.utils.LanceConstants.LANCE_TABLE
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
+import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
@@ -203,17 +204,26 @@ public class LanceTableOperations extends
ManagedTableOperations {
public boolean purgeTable(NameIdentifier ident) {
try {
Table table = loadTable(ident);
- String location = table.properties().get(Table.PROPERTY_LOCATION);
+ boolean external =
+ Optional.ofNullable(table.properties().get(Table.PROPERTY_EXTERNAL))
+ .map(Boolean::parseBoolean)
+ .orElse(false);
+ // NOTE: the two steps below (metadata removal and dataset deletion) are
NOT atomic.
+ // If the process crashes between them, the entity-store record will be
gone but the
+ // Lance dataset will still exist on storage (orphaned data), or
vice-versa. There is
+ // currently no recovery mechanism for this window.
boolean purged = super.purgeTable(ident);
+ // If the table is a managed table, super.purgeTable will call dropTable
to remove the
+ // underlying Lance dataset, so we don't need to do anything here.
+ if (!external) {
+ return purged;
+ }
+
// If the table metadata is purged successfully, we can delete the Lance
dataset.
// Otherwise, we should not delete the dataset.
if (purged) {
- // Delete the Lance dataset at the location
- Dataset.drop(
- location,
- LancePropertiesUtils.resolveLanceStorageOptions(catalogProperties,
table.properties()));
- LOG.info("Deleted Lance dataset at location {}", location);
+ dropLanceDataset(table);
}
return purged;
@@ -234,22 +244,19 @@ public class LanceTableOperations extends
ManagedTableOperations {
.map(Boolean::parseBoolean)
.orElse(false);
+ // NOTE: the two steps below (metadata removal and dataset deletion) are
NOT atomic.
+ // If the process crashes between them, the entity-store record will be
gone but the
+ // Lance dataset will still exist on storage (orphaned data), or
vice-versa. There is
+ // currently no recovery mechanism for this window.
boolean dropped = super.dropTable(ident);
if (external) {
return dropped;
}
- // If the table metadata is dropped successfully, and the table is not
external, we can delete
- // the
- // Lance dataset. Otherwise, we should not delete the dataset.
+ // If the table metadata is dropped successfully, and the table is not
external, we can
+ // delete the Lance dataset. Otherwise, we should not delete the dataset.
if (dropped) {
- String location = table.properties().get(Table.PROPERTY_LOCATION);
-
- // Delete the Lance dataset at the location
- Dataset.drop(
- location,
- LancePropertiesUtils.resolveLanceStorageOptions(catalogProperties,
table.properties()));
- LOG.info("Deleted Lance dataset at location {}", location);
+ dropLanceDataset(table);
}
return dropped;
@@ -261,6 +268,25 @@ public class LanceTableOperations extends
ManagedTableOperations {
}
}
+ private void dropLanceDataset(Table table) {
+ String location = table.properties().get(Table.PROPERTY_LOCATION);
+ Map<String, String> resolvedStorageOptions =
+ LancePropertiesUtils.resolveLanceStorageOptions(catalogProperties,
table.properties());
+ try {
+ Dataset.drop(location, resolvedStorageOptions);
+ LOG.info("Deleted Lance dataset at location {}", location);
+ } catch (Exception e) {
+ // Dataset.drop (native) throws IOException with "Not found:" when path
doesn't exist.
+ if (e instanceof IOException
+ && e.getMessage() != null
+ && e.getMessage().contains("Not found:")) {
+ LOG.warn("Lance dataset at {} was already deleted, skipping.",
location);
+ } else {
+ throw new RuntimeException("Failed to delete Lance dataset at " +
location, e);
+ }
+ }
+ }
+
// Package-private for testing
Table createTableInternal(
NameIdentifier ident,
diff --git
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
index d7b2b5f218..f2dc40907a 100644
---
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
+++
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
@@ -153,6 +153,85 @@ public class CatalogGenericCatalogLanceIT extends BaseIT {
createSchema();
}
+ @Test
+ public void testPurgeTableWhenLocationMissing() throws IOException {
+ Column[] columns = createColumns();
+ String location = tempDirectory + "/" +
GravitinoITUtils.genRandomName(TABLE_PREFIX) + "/";
+ NameIdentifier identifier =
+ NameIdentifier.of(schemaName,
GravitinoITUtils.genRandomName(TABLE_PREFIX));
+ Map<String, String> properties = createProperties();
+ properties.put(Table.PROPERTY_TABLE_FORMAT, LANCE_TABLE_FORMAT);
+ properties.put(Table.PROPERTY_EXTERNAL, "true");
+ properties.put(Table.PROPERTY_LOCATION, location);
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ identifier, columns, TABLE_COMMENT, properties,
Transforms.EMPTY_TRANSFORM, null, null);
+ Assertions.assertTrue(new File(location).exists());
+
+ // Simulate missing data before purge
+ FileUtils.deleteDirectory(new File(location));
+ Assertions.assertFalse(new File(location).exists());
+
+ Assertions.assertDoesNotThrow(() ->
catalog.asTableCatalog().purgeTable(identifier));
+ Assertions.assertFalse(catalog.asTableCatalog().tableExists(identifier));
+ }
+
+ @Test
+ public void testPurgeExternalTableWithEmptyDirectory() {
+ Column[] columns = createColumns();
+ String location = tempDirectory + "/" +
GravitinoITUtils.genRandomName(TABLE_PREFIX) + "/";
+ NameIdentifier identifier =
+ NameIdentifier.of(schemaName,
GravitinoITUtils.genRandomName(TABLE_PREFIX));
+ Map<String, String> properties = createProperties();
+ properties.put(Table.PROPERTY_TABLE_FORMAT, LANCE_TABLE_FORMAT);
+ properties.put(Table.PROPERTY_EXTERNAL, "true");
+ properties.put(Table.PROPERTY_LOCATION, location);
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ identifier, columns, TABLE_COMMENT, properties,
Transforms.EMPTY_TRANSFORM, null, null);
+ // The directory is created but contains no Lance dataset files
+ Assertions.assertTrue(new File(location).exists());
+
+ Assertions.assertDoesNotThrow(() ->
catalog.asTableCatalog().purgeTable(identifier));
+ Assertions.assertFalse(catalog.asTableCatalog().tableExists(identifier));
+ }
+
+ @Test
+ public void testPurgeManagedTableWithAutoLocation() {
+ Column[] columns = createColumns();
+ NameIdentifier identifier =
+ NameIdentifier.of(schemaName,
GravitinoITUtils.genRandomName(TABLE_PREFIX));
+ String location = tempDirectory + "/" +
GravitinoITUtils.genRandomName(TABLE_PREFIX) + "/";
+
+ Map<String, String> properties = createProperties();
+ properties.put(Table.PROPERTY_TABLE_FORMAT, LANCE_TABLE_FORMAT);
+ properties.put(Table.PROPERTY_EXTERNAL, "false");
+ properties.put(Table.PROPERTY_LOCATION, location);
+
+ Table table =
+ catalog
+ .asTableCatalog()
+ .createTable(
+ identifier,
+ columns,
+ TABLE_COMMENT,
+ properties,
+ Transforms.EMPTY_TRANSFORM,
+ null,
+ null);
+ String resolvedLocation = table.properties().get(Table.PROPERTY_LOCATION);
+ Assertions.assertNotNull(resolvedLocation);
+ Assertions.assertTrue(new File(resolvedLocation).exists());
+
+ Assertions.assertDoesNotThrow(() ->
catalog.asTableCatalog().purgeTable(identifier));
+ Assertions.assertFalse(catalog.asTableCatalog().tableExists(identifier));
+ Assertions.assertFalse(new File(resolvedLocation).exists());
+ }
+
@Test
public void testCrateEmptyTable() {
String emptyTableName = GravitinoITUtils.genRandomName(TABLE_PREFIX);
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
index 40798407e4..061e61250b 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
@@ -882,7 +882,7 @@ public class LanceRESTServiceIT extends BaseIT {
// Drop the table
DropTableRequest dropTableRequest = new DropTableRequest();
dropTableRequest.setId(ids);
- Assertions.assertThrows(Exception.class, () ->
ns.dropTable(dropTableRequest));
+ Assertions.assertDoesNotThrow(() -> ns.dropTable(dropTableRequest));
// Describe the dropped table should fail
DescribeTableRequest describeTableRequest = new DescribeTableRequest();
@@ -1014,6 +1014,55 @@ public class LanceRESTServiceIT extends BaseIT {
return new TableApi(apiClient);
}
+ @Test
+ void testDropTableWhenLocationMissingOrNotCreated() {
+ catalog = createCatalog(CATALOG_NAME);
+ createSchema();
+
+ // Create table via register with non-existent location
+ List<String> registerIds = List.of(CATALOG_NAME, SCHEMA_NAME,
"register_missing_location");
+ String missingLocation = tempDir + "/" + "register_missing_location/";
+ RegisterTableRequest registerTableRequest = new RegisterTableRequest();
+ registerTableRequest.setId(registerIds);
+ registerTableRequest.setLocation(missingLocation);
+ registerTableRequest.setMode("create");
+ RegisterTableResponse registerTableResponse =
+ Assertions.assertDoesNotThrow(() ->
ns.registerTable(registerTableRequest));
+ Assertions.assertEquals(missingLocation,
registerTableResponse.getLocation());
+ Assertions.assertFalse(new File(missingLocation).exists());
+
+ DropTableRequest dropRegisteredTable = new DropTableRequest();
+ dropRegisteredTable.setId(registerIds);
+ Assertions.assertDoesNotThrow(() -> ns.dropTable(dropRegisteredTable));
+ DescribeTableRequest describeRegistered = new DescribeTableRequest();
+ describeRegistered.setId(registerIds);
+ Assertions.assertThrows(
+ LanceNamespaceException.class, () ->
ns.describeTable(describeRegistered));
+
+ // Register table with an existing location, then delete the location
before drop
+ List<String> existingIds = List.of(CATALOG_NAME, SCHEMA_NAME,
"existing_then_deleted");
+ String existingLocation = tempDir + "/" + "existing_then_deleted/";
+ new File(existingLocation).mkdirs();
+ Assertions.assertTrue(new File(existingLocation).exists());
+
+ RegisterTableRequest existingRegisterRequest = new RegisterTableRequest();
+ existingRegisterRequest.setId(existingIds);
+ existingRegisterRequest.setLocation(existingLocation);
+ existingRegisterRequest.setMode("create");
+ Assertions.assertDoesNotThrow(() ->
ns.registerTable(existingRegisterRequest));
+
+ FileUtils.deleteQuietly(new File(existingLocation));
+ Assertions.assertFalse(new File(existingLocation).exists());
+
+ DropTableRequest dropExistingTable = new DropTableRequest();
+ dropExistingTable.setId(existingIds);
+ Assertions.assertDoesNotThrow(() -> ns.dropTable(dropExistingTable));
+ DescribeTableRequest describeExisting = new DescribeTableRequest();
+ describeExisting.setId(existingIds);
+ Assertions.assertThrows(
+ LanceNamespaceException.class, () ->
ns.describeTable(describeExisting));
+ }
+
private GravitinoMetalake createMetalake(String metalakeName) {
return client.createMetalake(metalakeName, "metalake for lance rest
service tests", null);
}
