This is an automated email from the ASF dual-hosted git repository.
yuqi4733 pushed a commit to branch branch-lance-namespace-dev
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-lance-namespace-dev by
this push:
new fe2efa77fd [#8939] Improvement(lance-catalog): Make operating
Gravitino store and Lance dataset atomic. (#9103)
fe2efa77fd is described below
commit fe2efa77fd65ee65fb82bc960a69755883d8e889
Author: Mini Yu <[email protected]>
AuthorDate: Thu Nov 13 17:11:46 2025 +0800
[#8939] Improvement(lance-catalog): Make operating Gravitino store and
Lance dataset atomic. (#9103)
### What changes were proposed in this pull request?
If we failed to create the Lance dataset, we need to remove the data
store in Gravitino store.
### Why are the changes needed?
Once we failed to create a lance table, without this PR, we can't try
again, as the metadata is already exists in the Graivitno server.
Fix: #8939
### Does this PR introduce _any_ user-facing change?
N/A
### How was this patch tested?
ITs.
---
.../GenericLakehouseCatalogOperations.java | 78 +++++++++++++---------
.../test/CatalogGenericLakehouseLanceIT.java | 30 +++++++++
.../lance/integration/test/LanceRESTServiceIT.java | 20 ++++++
3 files changed, 97 insertions(+), 31 deletions(-)
diff --git
a/catalogs/catalog-generic-lakehouse/src/main/java/org/apache/gravitino/catalog/lakehouse/GenericLakehouseCatalogOperations.java
b/catalogs/catalog-generic-lakehouse/src/main/java/org/apache/gravitino/catalog/lakehouse/GenericLakehouseCatalogOperations.java
index 5603ab1f22..6428fb816e 100644
---
a/catalogs/catalog-generic-lakehouse/src/main/java/org/apache/gravitino/catalog/lakehouse/GenericLakehouseCatalogOperations.java
+++
b/catalogs/catalog-generic-lakehouse/src/main/java/org/apache/gravitino/catalog/lakehouse/GenericLakehouseCatalogOperations.java
@@ -255,9 +255,8 @@ public class GenericLakehouseCatalogOperations
i -> ColumnEntity.toColumnEntity(columns[i], i,
idGenerator.nextId(), auditInfo))
.collect(Collectors.toList());
- TableEntity entityToStore;
try {
- entityToStore =
+ TableEntity entityToStore =
TableEntity.builder()
.withName(ident.name())
.withNamespace(ident.namespace())
@@ -273,41 +272,58 @@ public class GenericLakehouseCatalogOperations
.withAuditInfo(auditInfo)
.build();
store.put(entityToStore);
+ } catch (EntityAlreadyExistsException e) {
+ throw new TableAlreadyExistsException(e, "Table %s already exists in the
metadata", ident);
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to create table metadata for " +
ident, e);
+ }
- // Get the value of register in table properties
- boolean register =
- Boolean.parseBoolean(
- properties.getOrDefault(
- GenericLakehouseTablePropertiesMetadata.LAKEHOUSE_REGISTER,
"false"));
- if (register) {
- // Do not need to create the physical table if this is a registration
operation.
- // Whether we need to check the existence of the physical table?
- GenericLakehouseTable.Builder builder =
GenericLakehouseTable.builder();
- return builder
- .withName(ident.name())
- .withColumns(columns)
- .withComment(comment)
- .withProperties(properties)
- .withDistribution(distribution)
- .withIndexes(indexes)
- .withAuditInfo(
- AuditInfo.builder()
- .withCreator(PrincipalUtils.getCurrentUserName())
- .withCreateTime(Instant.now())
- .build())
- .withPartitioning(partitions)
- .withSortOrders(sortOrders)
- .withFormat(LakehouseTableFormat.LANCE.lowerName())
- .build();
- }
+ // Get the value of register in table properties
+ boolean register =
+ Boolean.parseBoolean(
+ properties.getOrDefault(
+ GenericLakehouseTablePropertiesMetadata.LAKEHOUSE_REGISTER,
"false"));
+ if (register) {
+ // Do not need to create the physical table if this is a registration
operation.
+ // Whether we need to check the existence of the physical table?
+ GenericLakehouseTable.Builder builder = GenericLakehouseTable.builder();
+ return builder
+ .withName(ident.name())
+ .withColumns(columns)
+ .withComment(comment)
+ .withProperties(properties)
+ .withDistribution(distribution)
+ .withIndexes(indexes)
+ .withAuditInfo(
+ AuditInfo.builder()
+ .withCreator(PrincipalUtils.getCurrentUserName())
+ .withCreateTime(Instant.now())
+ .build())
+ .withPartitioning(partitions)
+ .withSortOrders(sortOrders)
+ .withFormat(LakehouseTableFormat.LANCE.lowerName())
+ .build();
+ }
+ try {
LakehouseCatalogOperations lanceCatalogOperations =
getLakehouseCatalogOperations(newProperties);
return lanceCatalogOperations.createTable(
ident, columns, comment, newProperties, partitions, distribution,
sortOrders, indexes);
- } catch (EntityAlreadyExistsException e) {
- throw new TableAlreadyExistsException(e, "Table %s already exists",
ident);
- } catch (IOException e) {
+ } catch (Exception e) {
+ // Try to roll back the metadata entry in Gravitino store
+ try {
+ store.delete(ident, Entity.EntityType.TABLE);
+ } catch (IOException ioException) {
+ LOG.error(
+ "Failed to roll back the metadata entry for table {} after
physical table creation failure.",
+ ident,
+ ioException);
+ }
+ if (e.getClass().isAssignableFrom(RuntimeException.class)) {
+ throw e;
+ }
+
throw new RuntimeException("Failed to create table " + ident, e);
}
}
diff --git
a/catalogs/catalog-generic-lakehouse/src/test/java/org/apache/gravitino/catalog/lakehouse/integration/test/CatalogGenericLakehouseLanceIT.java
b/catalogs/catalog-generic-lakehouse/src/test/java/org/apache/gravitino/catalog/lakehouse/integration/test/CatalogGenericLakehouseLanceIT.java
index ca1e8ca003..0c92b43eef 100644
---
a/catalogs/catalog-generic-lakehouse/src/test/java/org/apache/gravitino/catalog/lakehouse/integration/test/CatalogGenericLakehouseLanceIT.java
+++
b/catalogs/catalog-generic-lakehouse/src/test/java/org/apache/gravitino/catalog/lakehouse/integration/test/CatalogGenericLakehouseLanceIT.java
@@ -53,6 +53,7 @@ import org.apache.gravitino.Catalog;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Schema;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.integration.test.util.BaseIT;
import org.apache.gravitino.integration.test.util.GravitinoITUtils;
import org.apache.gravitino.rel.Column;
@@ -255,6 +256,35 @@ public class CatalogGenericLakehouseLanceIT extends BaseIT
{
Arrays.asList(catalog.asTableCatalog().listTables(nameIdentifier.namespace()));
Assertions.assertEquals(1, tableIdentifiers.size());
Assertions.assertEquals(nameIdentifier, tableIdentifiers.get(0));
+
+ // Now try to simulate the location of lance table does not exist.
+ Map<String, String> newProperties = createProperties();
+ newProperties.put("format", "lance");
+ // Use a wrong location to let the table creation fail
+ newProperties.put("location", "hdfs://localhost:9000/wrong_location");
+
+ String nameNew = GravitinoITUtils.genRandomName(TABLE_PREFIX);
+ NameIdentifier newNameIdentifier = NameIdentifier.of(schemaName, nameNew);
+ Exception e =
+ Assertions.assertThrows(
+ Exception.class,
+ () -> {
+ catalog
+ .asTableCatalog()
+ .createTable(
+ newNameIdentifier,
+ columns,
+ TABLE_COMMENT,
+ newProperties,
+ Transforms.EMPTY_TRANSFORM,
+ null,
+ null);
+ });
+
+ Assertions.assertTrue(e.getMessage().contains("Invalid user input"));
+
+ Assertions.assertThrows(
+ NoSuchTableException.class, () ->
catalog.asTableCatalog().loadTable(newNameIdentifier));
}
@Test
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 c7454442b6..a63bb39307 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
@@ -440,6 +440,26 @@ public class LanceRESTServiceIT extends BaseIT {
});
Assertions.assertTrue(exception.getMessage().contains("Table already
exists"));
Assertions.assertEquals(409, exception.getCode());
+
+ // Try to create a table with wrong location should fail
+ CreateEmptyTableRequest wrongLocationRequest = new
CreateEmptyTableRequest();
+ wrongLocationRequest.setId(List.of(CATALOG_NAME, SCHEMA_NAME,
"wrong_location_table"));
+ wrongLocationRequest.setLocation("hdfs://localhost:9000/invalid_path/");
+ LanceNamespaceException apiException =
+ Assertions.assertThrows(
+ LanceNamespaceException.class,
+ () -> {
+ ns.createEmptyTable(wrongLocationRequest);
+ });
+ Assertions.assertTrue(apiException.getMessage().contains("Invalid user
input"));
+
+ // Correct the location and try again
+ String correctedLocation = tempDir + "/" + "wrong_location_table/";
+ wrongLocationRequest.setLocation(correctedLocation);
+ CreateEmptyTableResponse wrongLocationResponse =
+ Assertions.assertDoesNotThrow(() ->
ns.createEmptyTable(wrongLocationRequest));
+ Assertions.assertNotNull(wrongLocationResponse);
+ Assertions.assertEquals(correctedLocation,
wrongLocationResponse.getLocation());
}
@Test
