From 472a791f4a8fe2fa33e41673bb00c254a89817b3 Mon Sep 17 00:00:00 2001 From: yuqi Date: Mon, 3 Aug 2026 22:27:11 +0800 Subject: [PATCH 1/9] [#12342] improvement(core): add OCC for namespace hierarchy --- .../relational/mapper/CatalogMetaMapper.java | 3 +- .../mapper/CatalogMetaSQLProviderFactory.java | 5 +- .../relational/mapper/MetalakeMetaMapper.java | 3 +- .../MetalakeMetaSQLProviderFactory.java | 5 +- .../relational/mapper/SchemaMetaMapper.java | 6 ++ .../mapper/SchemaMetaSQLProviderFactory.java | 5 ++ .../base/CatalogMetaBaseSQLProvider.java | 15 +--- .../base/MetalakeMetaBaseSQLProvider.java | 13 +-- .../base/SchemaMetaBaseSQLProvider.java | 18 ++-- .../CatalogMetaPostgreSQLProvider.java | 15 +--- .../MetalakeMetaPostgreSQLProvider.java | 13 +-- .../SchemaMetaPostgreSQLProvider.java | 18 ++-- .../service/CatalogMetaService.java | 59 ++++++++----- .../service/MetalakeMetaService.java | 59 +++++++++---- .../relational/service/SchemaMetaService.java | 61 ++++++++----- .../relational/utils/POConverters.java | 6 +- .../service/TestCatalogMetaService.java | 51 +++++++++++ .../service/TestMetalakeMetaService.java | 88 +++++++++++++++++++ .../service/TestSchemaMetaService.java | 54 ++++++++++++ .../relational/utils/TestPOConverters.java | 6 ++ 20 files changed, 372 insertions(+), 131 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java index 9f19d1a6a84..f1f1f3231ab 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java @@ -89,7 +89,8 @@ Integer updateCatalogMeta( @UpdateProvider( type = CatalogMetaSQLProviderFactory.class, method = "softDeleteCatalogMetasByCatalogId") - Integer softDeleteCatalogMetasByCatalogId(@Param("catalogId") Long catalogId); + Integer softDeleteCatalogMetasByCatalogId( + @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion); @UpdateProvider( type = CatalogMetaSQLProviderFactory.class, diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java index c3a7954a25a..506c7874b9b 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java @@ -109,8 +109,9 @@ public static String updateCatalogMeta( return getProvider().updateCatalogMeta(newCatalogPO, oldCatalogPO); } - public static String softDeleteCatalogMetasByCatalogId(@Param("catalogId") Long catalogId) { - return getProvider().softDeleteCatalogMetasByCatalogId(catalogId); + public static String softDeleteCatalogMetasByCatalogId( + @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion) { + return getProvider().softDeleteCatalogMetasByCatalogId(catalogId, currentVersion); } public static String softDeleteCatalogMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java index f705c283ce6..33787a71a59 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java @@ -73,7 +73,8 @@ Integer updateMetalakeMeta( @UpdateProvider( type = MetalakeMetaSQLProviderFactory.class, method = "softDeleteMetalakeMetaByMetalakeId") - Integer softDeleteMetalakeMetaByMetalakeId(@Param("metalakeId") Long metalakeId); + Integer softDeleteMetalakeMetaByMetalakeId( + @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion); @DeleteProvider( type = MetalakeMetaSQLProviderFactory.class, diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java index eba26f9e025..6d95b3a1e43 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java @@ -88,8 +88,9 @@ public static String updateMetalakeMeta( return getProvider().updateMetalakeMeta(newMetalakePO, oldMetalakePO); } - public static String softDeleteMetalakeMetaByMetalakeId(@Param("metalakeId") Long metalakeId) { - return getProvider().softDeleteMetalakeMetaByMetalakeId(metalakeId); + public static String softDeleteMetalakeMetaByMetalakeId( + @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { + return getProvider().softDeleteMetalakeMetaByMetalakeId(metalakeId, currentVersion); } public static String deleteMetalakeMetasByLegacyTimeline( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java index 1c9b5286b29..e8310e977c0 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java @@ -107,6 +107,12 @@ Integer updateSchemaMeta( method = "softDeleteSchemaMetasBySchemaIds") Integer softDeleteSchemaMetasBySchemaIds(@Param("schemaIds") List schemaIds); + @UpdateProvider( + type = SchemaMetaSQLProviderFactory.class, + method = "softDeleteSchemaMetaBySchemaIdAndVersion") + Integer softDeleteSchemaMetaBySchemaIdAndVersion( + @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion); + @UpdateProvider( type = SchemaMetaSQLProviderFactory.class, method = "softDeleteSchemaMetasByMetalakeId") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java index acc27170269..c65b23ad142 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java @@ -120,6 +120,11 @@ public static String softDeleteSchemaMetasBySchemaIds(@Param("schemaIds") List sc + ""; } + public String softDeleteSchemaMetaBySchemaIdAndVersion( + @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion) { + return "UPDATE " + + TABLE_NAME + + " SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)" + + " + EXTRACT(MICROSECOND FROM CURRENT_TIMESTAMP(3)) / 1000" + + " WHERE schema_id = #{schemaId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; + } + public String softDeleteSchemaMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { return "UPDATE " + TABLE_NAME diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java index 0482d9b330b..79f704162b8 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java @@ -26,11 +26,12 @@ public class CatalogMetaPostgreSQLProvider extends CatalogMetaBaseSQLProvider { @Override - public String softDeleteCatalogMetasByCatalogId(Long catalogId) { + public String softDeleteCatalogMetasByCatalogId(Long catalogId, Long currentVersion) { return "UPDATE " + TABLE_NAME + " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000 AS BIGINT)" - + " WHERE catalog_id = #{catalogId} AND deleted_at = 0"; + + " WHERE catalog_id = #{catalogId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; } @Override @@ -101,17 +102,7 @@ public String updateCatalogMeta( + " last_version = #{newCatalogMeta.lastVersion}," + " deleted_at = #{newCatalogMeta.deletedAt}" + " WHERE catalog_id = #{oldCatalogMeta.catalogId}" - + " AND catalog_name = #{oldCatalogMeta.catalogName}" - + " AND metalake_id = #{oldCatalogMeta.metalakeId}" - + " AND type = #{oldCatalogMeta.type}" - + " AND provider = #{oldCatalogMeta.provider}" - + " AND (catalog_comment = #{oldCatalogMeta.catalogComment} " - + " OR (CAST(catalog_comment AS VARCHAR) IS NULL AND " - + " CAST(#{oldCatalogMeta.catalogComment} AS VARCHAR) IS NULL))" - + " AND properties = #{oldCatalogMeta.properties}" - + " AND audit_info = #{oldCatalogMeta.auditInfo}" + " AND current_version = #{oldCatalogMeta.currentVersion}" - + " AND last_version = #{oldCatalogMeta.lastVersion}" + " AND deleted_at = 0"; } } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/MetalakeMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/MetalakeMetaPostgreSQLProvider.java index 5ce01e67159..20a92d1063c 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/MetalakeMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/MetalakeMetaPostgreSQLProvider.java @@ -26,11 +26,12 @@ public class MetalakeMetaPostgreSQLProvider extends MetalakeMetaBaseSQLProvider { @Override - public String softDeleteMetalakeMetaByMetalakeId(Long metalakeId) { + public String softDeleteMetalakeMetaByMetalakeId(Long metalakeId, Long currentVersion) { return "UPDATE " + TABLE_NAME + " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000 AS BIGINT)" - + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; + + " WHERE metalake_id = #{metalakeId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; } @Override @@ -75,15 +76,7 @@ public String updateMetalakeMeta( + " current_version = #{newMetalakeMeta.currentVersion}," + " last_version = #{newMetalakeMeta.lastVersion}" + " WHERE metalake_id = #{oldMetalakeMeta.metalakeId}" - + " AND metalake_name = #{oldMetalakeMeta.metalakeName}" - + " AND (metalake_comment = #{oldMetalakeMeta.metalakeComment} " - + " OR (CAST(metalake_comment AS VARCHAR) IS NULL AND " - + " CAST(#{oldMetalakeMeta.metalakeComment} AS VARCHAR) IS NULL))" - + " AND properties = #{oldMetalakeMeta.properties}" - + " AND audit_info = #{oldMetalakeMeta.auditInfo}" - + " AND schema_version = #{oldMetalakeMeta.schemaVersion}" + " AND current_version = #{oldMetalakeMeta.currentVersion}" - + " AND last_version = #{oldMetalakeMeta.lastVersion}" + " AND deleted_at = 0"; } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java index ba2087aa61a..e2e6f9557e8 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java @@ -98,16 +98,7 @@ public String updateSchemaMeta( + " last_version = #{newSchemaMeta.lastVersion}," + " deleted_at = #{newSchemaMeta.deletedAt}" + " WHERE schema_id = #{oldSchemaMeta.schemaId}" - + " AND schema_name = #{oldSchemaMeta.schemaName}" - + " AND metalake_id = #{oldSchemaMeta.metalakeId}" - + " AND catalog_id = #{oldSchemaMeta.catalogId}" - + " AND (schema_comment = #{oldSchemaMeta.schemaComment}" - + " OR (CAST(schema_comment AS VARCHAR) IS NULL" - + " AND CAST(#{oldSchemaMeta.schemaComment} AS VARCHAR) IS NULL))" - + " AND properties = #{oldSchemaMeta.properties}" - + " AND audit_info = #{oldSchemaMeta.auditInfo}" + " AND current_version = #{oldSchemaMeta.currentVersion}" - + " AND last_version = #{oldSchemaMeta.lastVersion}" + " AND deleted_at = 0"; } @@ -125,6 +116,15 @@ public String softDeleteSchemaMetasBySchemaIds(List schemaIds) { + ""; } + @Override + public String softDeleteSchemaMetaBySchemaIdAndVersion(Long schemaId, Long currentVersion) { + return "UPDATE " + + TABLE_NAME + + " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000 AS BIGINT)" + + " WHERE schema_id = #{schemaId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; + } + @Override public String softDeleteSchemaMetasByMetalakeId(Long metalakeId) { return "UPDATE " diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java index a85e1b85bad..c4791c8c395 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java @@ -34,6 +34,7 @@ import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NonEmptyEntityException; +import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.CatalogEntity; import org.apache.gravitino.meta.SchemaEntity; import org.apache.gravitino.metrics.Monitored; @@ -229,15 +230,19 @@ public CatalogEntity updateCatalog( AtomicInteger updateResult = new AtomicInteger(0); try { SessionUtils.doMultipleWithCommit( - () -> - updateResult.set( - SessionUtils.getWithoutCommit( - CatalogMetaMapper.class, - mapper -> - mapper.updateCatalogMeta( - POConverters.updateCatalogPOWithVersion( - oldCatalogPO, newEntity, oldCatalogPO.getMetalakeId()), - oldCatalogPO))), + () -> { + updateResult.set( + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> + mapper.updateCatalogMeta( + POConverters.updateCatalogPOWithVersion( + oldCatalogPO, newEntity, oldCatalogPO.getMetalakeId()), + oldCatalogPO))); + if (updateResult.get() == 0) { + throw optimisticLockException(identifier); + } + }, () -> { if (updateResult.get() > 0) { SessionUtils.doWithoutCommit( @@ -256,11 +261,7 @@ public CatalogEntity updateCatalog( throw re; } - if (updateResult.get() > 0) { - return newEntity; - } else { - throw new IOException("Failed to update the entity: " + identifier); - } + return newEntity; } @Monitored( @@ -270,15 +271,14 @@ public boolean deleteCatalog(NameIdentifier identifier, boolean cascade) { NameIdentifierUtil.checkCatalog(identifier); String catalogName = identifier.name(); - long catalogId = EntityIdService.getEntityId(identifier, Entity.EntityType.CATALOG); + CatalogPO catalogPO = getCatalogPOByName(identifier.namespace().level(0), catalogName); + long catalogId = catalogPO.getCatalogId(); + long currentVersion = catalogPO.getCurrentVersion(); String metalakeName = identifier.namespace().level(0); if (cascade) { SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - CatalogMetaMapper.class, - mapper -> mapper.softDeleteCatalogMetasByCatalogId(catalogId)), + () -> deleteCatalogWithVersion(identifier, catalogId, currentVersion), () -> SessionUtils.doWithoutCommit( SchemaMetaMapper.class, @@ -368,10 +368,7 @@ public boolean deleteCatalog(NameIdentifier identifier, boolean cascade) { "Entity %s has sub-entities, you should remove sub-entities first", identifier); } SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - CatalogMetaMapper.class, - mapper -> mapper.softDeleteCatalogMetasByCatalogId(catalogId)), + () -> deleteCatalogWithVersion(identifier, catalogId, currentVersion), () -> SessionUtils.doWithoutCommit( OwnerMetaMapper.class, @@ -415,6 +412,22 @@ public boolean deleteCatalog(NameIdentifier identifier, boolean cascade) { return true; } + private void deleteCatalogWithVersion( + NameIdentifier identifier, Long catalogId, Long currentVersion) { + int deleted = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.softDeleteCatalogMetasByCatalogId(catalogId, currentVersion)); + if (deleted == 0) { + throw optimisticLockException(identifier); + } + } + + private OptimisticLockException optimisticLockException(NameIdentifier identifier) { + return new OptimisticLockException( + "The catalog %s was modified concurrently; retry the operation", identifier); + } + @Monitored( metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME, baseMetricName = "deleteCatalogMetasByLegacyTimeline") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java index 607dab00c11..968456844e1 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java @@ -33,6 +33,7 @@ import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NonEmptyEntityException; +import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.BaseMetalake; import org.apache.gravitino.meta.CatalogEntity; import org.apache.gravitino.metrics.Monitored; @@ -183,11 +184,15 @@ public BaseMetalake updateMetalake( AtomicInteger updateResult = new AtomicInteger(0); try { SessionUtils.doMultipleWithCommit( - () -> - updateResult.set( - SessionUtils.getWithoutCommit( - MetalakeMetaMapper.class, - mapper -> mapper.updateMetalakeMeta(newMetalakePO, oldMetalakePO))), + () -> { + updateResult.set( + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, + mapper -> mapper.updateMetalakeMeta(newMetalakePO, oldMetalakePO))); + if (updateResult.get() == 0) { + throw optimisticLockException(ident); + } + }, () -> { if (isRenamed && updateResult.get() > 0) { SessionUtils.doWithoutCommit( @@ -206,11 +211,7 @@ public BaseMetalake updateMetalake( throw re; } - if (updateResult.get() > 0) { - return newMetalakeEntity; - } else { - throw new IOException("Failed to update the entity: " + ident); - } + return newMetalakeEntity; } @Monitored( @@ -218,14 +219,21 @@ public BaseMetalake updateMetalake( baseMetricName = "deleteMetalake") public boolean deleteMetalake(NameIdentifier ident, boolean cascade) { NameIdentifierUtil.checkMetalake(ident); - Long metalakeId = getMetalakeIdByName(ident.name()); + MetalakePO metalakePO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(ident.name())); + if (metalakePO == null) { + throw new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.METALAKE.name().toLowerCase(), + ident.toString()); + } + Long metalakeId = metalakePO.getMetalakeId(); + Long currentVersion = metalakePO.getCurrentVersion(); if (metalakeId != null) { if (cascade) { SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - MetalakeMetaMapper.class, - mapper -> mapper.softDeleteMetalakeMetaByMetalakeId(metalakeId)), + () -> deleteMetalakeWithVersion(ident, metalakeId, currentVersion), () -> SessionUtils.doWithoutCommit( CatalogMetaMapper.class, @@ -353,10 +361,7 @@ public boolean deleteMetalake(NameIdentifier ident, boolean cascade) { "Entity %s has sub-entities, you should remove sub-entities first", ident); } SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - MetalakeMetaMapper.class, - mapper -> mapper.softDeleteMetalakeMetaByMetalakeId(metalakeId)), + () -> deleteMetalakeWithVersion(ident, metalakeId, currentVersion), () -> SessionUtils.doWithoutCommit( UserRoleRelMapper.class, @@ -420,6 +425,22 @@ public boolean deleteMetalake(NameIdentifier ident, boolean cascade) { return true; } + private void deleteMetalakeWithVersion( + NameIdentifier identifier, Long metalakeId, Long currentVersion) { + int deleted = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, + mapper -> mapper.softDeleteMetalakeMetaByMetalakeId(metalakeId, currentVersion)); + if (deleted == 0) { + throw optimisticLockException(identifier); + } + } + + private OptimisticLockException optimisticLockException(NameIdentifier identifier) { + return new OptimisticLockException( + "The metalake %s was modified concurrently; retry the operation", identifier); + } + @Monitored( metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME, baseMetricName = "deleteMetalakeMetasByLegacyTimeline") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java index 39d820b42d0..aa17ec6f4bc 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java @@ -40,6 +40,7 @@ import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NonEmptyEntityException; +import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.FilesetEntity; import org.apache.gravitino.meta.ModelEntity; import org.apache.gravitino.meta.NamespacedEntityId; @@ -230,15 +231,19 @@ public SchemaEntity updateSchema( AtomicInteger updateResult = new AtomicInteger(0); try { SessionUtils.doMultipleWithCommit( - () -> - updateResult.set( - SessionUtils.getWithoutCommit( - SchemaMetaMapper.class, - mapper -> - ops.updatePO( - mapper, - POConverters.updateSchemaPOWithVersion(oldSchemaPO, newEntity), - oldSchemaPO))), + () -> { + updateResult.set( + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + ops.updatePO( + mapper, + POConverters.updateSchemaPOWithVersion(oldSchemaPO, newEntity), + oldSchemaPO))); + if (updateResult.get() == 0) { + throw optimisticLockException(identifier); + } + }, () -> { if (isRenamed && updateResult.get() > 0) { SessionUtils.doWithoutCommit( @@ -257,11 +262,7 @@ public SchemaEntity updateSchema( throw re; } - if (updateResult.get() > 0) { - return newEntity; - } else { - throw new IOException("Failed to update the entity: " + identifier); - } + return newEntity; } @Monitored( @@ -287,11 +288,17 @@ public boolean deleteSchema(NameIdentifier identifier, boolean cascade) { if (schemaIds.isEmpty()) { return false; } + List descendantSchemaIds = + schemaIds.stream().filter(id -> !id.equals(schemaId)).collect(Collectors.toList()); SessionUtils.doMultipleWithCommit( - () -> + () -> deleteSchemaWithVersion(identifier, schemaId, schemaPO.getCurrentVersion()), + () -> { + if (!descendantSchemaIds.isEmpty()) { SessionUtils.doWithoutCommit( SchemaMetaMapper.class, - mapper -> mapper.softDeleteSchemaMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteSchemaMetasBySchemaIds(descendantSchemaIds)); + } + }, () -> SessionUtils.doWithoutCommit( TableMetaMapper.class, @@ -411,12 +418,8 @@ public boolean deleteSchema(NameIdentifier identifier, boolean cascade) { "Entity %s has sub-entities, you should remove sub-entities first", identifier); } - List singleSchemaId = Collections.singletonList(schemaId); SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - SchemaMetaMapper.class, - mapper -> mapper.softDeleteSchemaMetasBySchemaIds(singleSchemaId)), + () -> deleteSchemaWithVersion(identifier, schemaId, schemaPO.getCurrentVersion()), () -> SessionUtils.doWithoutCommit( OwnerMetaMapper.class, @@ -459,6 +462,22 @@ public boolean deleteSchema(NameIdentifier identifier, boolean cascade) { return true; } + private void deleteSchemaWithVersion( + NameIdentifier identifier, Long schemaId, Long currentVersion) { + int deleted = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> mapper.softDeleteSchemaMetaBySchemaIdAndVersion(schemaId, currentVersion)); + if (deleted == 0) { + throw optimisticLockException(identifier); + } + } + + private OptimisticLockException optimisticLockException(NameIdentifier identifier) { + return new OptimisticLockException( + "The schema %s was modified concurrently; retry the operation", identifier); + } + @Monitored( metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME, baseMetricName = "deleteSchemaMetasByLegacyTimeline") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java b/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java index fcb05a6f466..16a274ce05c 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java @@ -138,7 +138,7 @@ public static MetalakePO updateMetalakePOWithVersion( MetalakePO oldMetalakePO, BaseMetalake newMetalake) { Long lastVersion = oldMetalakePO.getLastVersion(); // Will set the version to the last version + 1 when having some fields need be multiple version - Long nextVersion = lastVersion; + Long nextVersion = lastVersion + 1; try { return MetalakePO.builder() .withMetalakeId(newMetalake.id()) @@ -235,7 +235,7 @@ public static CatalogPO updateCatalogPOWithVersion( CatalogPO oldCatalogPO, CatalogEntity newCatalog, Long metalakeId) { Long lastVersion = oldCatalogPO.getLastVersion(); // Will set the version to the last version + 1 when having some fields need be multiple version - Long nextVersion = lastVersion; + Long nextVersion = lastVersion + 1; try { return CatalogPO.builder() .withCatalogId(newCatalog.id()) @@ -331,7 +331,7 @@ public static SchemaPO initializeSchemaPOWithVersion( public static SchemaPO updateSchemaPOWithVersion(SchemaPO oldSchemaPO, SchemaEntity newSchema) { Long lastVersion = oldSchemaPO.getLastVersion(); // Will set the version to the last version + 1 when having some fields need be multiple version - Long nextVersion = lastVersion; + Long nextVersion = lastVersion + 1; try { return SchemaPO.builder() .withSchemaId(oldSchemaPO.getSchemaId()) diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java index de5520908af..c64f07bd1fc 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java @@ -50,7 +50,9 @@ import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.TestJDBCBackend; import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; +import org.apache.gravitino.storage.relational.po.CatalogPO; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; +import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; import org.apache.gravitino.utils.NameIdentifierUtil; import org.apache.gravitino.utils.NamespaceUtil; @@ -149,6 +151,55 @@ void testUpdateCatalogWithNullableComment() throws IOException { Assertions.assertNotNull(updatedCatalog.getComment()); } + @TestTemplate + public void testAlterAndDeleteUseCurrentVersion() throws IOException { + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "catalog_occ", + auditInfo); + backend.insert(catalog, false); + CatalogPO oldPO = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + CatalogEntity updatedCatalog = + CatalogEntity.builder() + .withId(catalog.id()) + .withName(catalog.name()) + .withNamespace(catalog.namespace()) + .withAuditInfo(auditInfo) + .withComment("updated") + .withProperties(catalog.getProperties()) + .withType(catalog.getType()) + .withProvider(catalog.getProvider()) + .build(); + CatalogPO newPO = + POConverters.updateCatalogPOWithVersion(oldPO, updatedCatalog, oldPO.getMetalakeId()); + + int updated = + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, mapper -> mapper.updateCatalogMeta(newPO, oldPO)); + int staleUpdate = + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, mapper -> mapper.updateCatalogMeta(newPO, oldPO)); + int staleDelete = + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, + mapper -> + mapper.softDeleteCatalogMetasByCatalogId(catalog.id(), oldPO.getCurrentVersion())); + assertEquals(1, updated); + assertEquals(0, staleUpdate); + assertEquals(0, staleDelete); + assertTrue(backend.exists(catalog.nameIdentifier(), Entity.EntityType.CATALOG)); + int deleted = + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, + mapper -> + mapper.softDeleteCatalogMetasByCatalogId(catalog.id(), newPO.getCurrentVersion())); + assertEquals(1, deleted); + } + @TestTemplate public void testMetaLifeCycleFromCreationToDeletion() throws IOException { CatalogEntity catalog = diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java index b1e6389a208..618263479fb 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java @@ -27,10 +27,15 @@ import java.util.List; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; +import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.BaseMetalake; import org.apache.gravitino.meta.SchemaVersion; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.TestJDBCBackend; +import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper; +import org.apache.gravitino.storage.relational.po.MetalakePO; +import org.apache.gravitino.storage.relational.utils.POConverters; +import org.apache.gravitino.storage.relational.utils.SessionUtils; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.TestTemplate; @@ -92,6 +97,89 @@ void testUpdateMetalakeWithNullableComment() throws IOException { backend.delete(metalake.nameIdentifier(), Entity.EntityType.METALAKE, false); } + @TestTemplate + public void testAlterAndDeleteUseCurrentVersion() throws IOException { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + MetalakePO oldPO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalake.name())); + BaseMetalake updatedMetalake = + BaseMetalake.builder() + .withId(metalake.id()) + .withName(metalake.name()) + .withAuditInfo(metalake.auditInfo()) + .withComment("updated") + .withProperties(metalake.properties()) + .withVersion(metalake.getVersion()) + .build(); + MetalakePO newPO = POConverters.updateMetalakePOWithVersion(oldPO, updatedMetalake); + + int updated = + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, mapper -> mapper.updateMetalakeMeta(newPO, oldPO)); + int staleUpdate = + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, mapper -> mapper.updateMetalakeMeta(newPO, oldPO)); + int staleDelete = + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, + mapper -> + mapper.softDeleteMetalakeMetaByMetalakeId( + metalake.id(), oldPO.getCurrentVersion())); + Assertions.assertEquals(1, updated); + Assertions.assertEquals(0, staleUpdate); + Assertions.assertEquals(0, staleDelete); + assertTrue(backend.exists(metalake.nameIdentifier(), Entity.EntityType.METALAKE)); + int deleted = + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, + mapper -> + mapper.softDeleteMetalakeMetaByMetalakeId( + metalake.id(), newPO.getCurrentVersion())); + Assertions.assertEquals(1, deleted); + } + + @TestTemplate + public void testAlterReportsOptimisticLockConflict() throws IOException { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + + assertThrows( + OptimisticLockException.class, + () -> + MetalakeMetaService.getInstance() + .updateMetalake( + metalake.nameIdentifier(), + entity -> { + BaseMetalake current = (BaseMetalake) entity; + MetalakePO currentPO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, + mapper -> mapper.selectMetalakeMetaByName(current.name())); + BaseMetalake competingUpdate = + BaseMetalake.builder() + .withId(current.id()) + .withName(current.name()) + .withAuditInfo(current.auditInfo()) + .withComment("competing update") + .withProperties(current.properties()) + .withVersion(current.getVersion()) + .build(); + MetalakePO competingPO = + POConverters.updateMetalakePOWithVersion(currentPO, competingUpdate); + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, + mapper -> mapper.updateMetalakeMeta(competingPO, currentPO)); + return BaseMetalake.builder() + .withId(current.id()) + .withName(current.name()) + .withAuditInfo(current.auditInfo()) + .withComment("requested update") + .withProperties(current.properties()) + .withVersion(current.getVersion()) + .build(); + })); + } + @TestTemplate public void testMetaLifeCycleFromCreationToDeletion() throws IOException { // meta data creation diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java index a1e43144ec8..e47507a3245 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java @@ -49,7 +49,11 @@ import org.apache.gravitino.rel.types.Types; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.TestJDBCBackend; +import org.apache.gravitino.storage.relational.mapper.SchemaMetaMapper; +import org.apache.gravitino.storage.relational.po.SchemaPO; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; +import org.apache.gravitino.storage.relational.utils.POConverters; +import org.apache.gravitino.storage.relational.utils.SessionUtils; import org.apache.gravitino.utils.NameIdentifierUtil; import org.apache.gravitino.utils.NamespaceUtil; import org.apache.ibatis.session.SqlSession; @@ -145,6 +149,56 @@ public void testUpdateSchemaCommentFromNull() throws IOException { Assertions.assertEquals("schema comment updated", updatedSchema.comment()); } + @TestTemplate + public void testAlterAndDeleteUseCurrentVersion() throws IOException { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "schema_occ", + AUDIT_INFO); + backend.insert(schema, false); + SchemaPO oldPO = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); + SchemaEntity updatedSchema = + SchemaEntity.builder() + .withId(schema.id()) + .withName(schema.name()) + .withNamespace(schema.namespace()) + .withAuditInfo(schema.auditInfo()) + .withComment("updated") + .withProperties(schema.properties()) + .build(); + SchemaPO newPO = POConverters.updateSchemaPOWithVersion(oldPO, updatedSchema); + + int updated = + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, mapper -> mapper.updateSchemaMeta(newPO, oldPO)); + int staleUpdate = + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, mapper -> mapper.updateSchemaMeta(newPO, oldPO)); + int staleDelete = + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, + mapper -> + mapper.softDeleteSchemaMetaBySchemaIdAndVersion( + schema.id(), oldPO.getCurrentVersion())); + Assertions.assertEquals(1, updated); + Assertions.assertEquals(0, staleUpdate); + Assertions.assertEquals(0, staleDelete); + assertTrue(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + int deleted = + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, + mapper -> + mapper.softDeleteSchemaMetaBySchemaIdAndVersion( + schema.id(), newPO.getCurrentVersion())); + Assertions.assertEquals(1, deleted); + } + @TestTemplate public void testMetaLifeCycleFromCreationToDeletion() throws IOException { createAndInsertMakeLake(metalakeName); diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestPOConverters.java b/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestPOConverters.java index bff96b2bd60..c32238af5ba 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestPOConverters.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestPOConverters.java @@ -665,6 +665,8 @@ public void testUpdateMetalakePOVersion() { assertEquals(1, initPO.getCurrentVersion()); assertEquals(1, initPO.getLastVersion()); assertEquals(0, initPO.getDeletedAt()); + assertEquals(2, updatePO.getCurrentVersion()); + assertEquals(2, updatePO.getLastVersion()); assertEquals("this is test2", updatePO.getMetalakeComment()); } @@ -679,6 +681,8 @@ public void testUpdateCatalogPOVersion() { assertEquals(1, initPO.getCurrentVersion()); assertEquals(1, initPO.getLastVersion()); assertEquals(0, initPO.getDeletedAt()); + assertEquals(2, updatePO.getCurrentVersion()); + assertEquals(2, updatePO.getLastVersion()); assertEquals("this is test2", updatePO.getCatalogComment()); } @@ -696,6 +700,8 @@ public void testUpdateSchemaPOVersion() { assertEquals(1, initPO.getCurrentVersion()); assertEquals(1, initPO.getLastVersion()); assertEquals(0, initPO.getDeletedAt()); + assertEquals(2, updatePO.getCurrentVersion()); + assertEquals(2, updatePO.getLastVersion()); assertEquals("this is test2", updatePO.getSchemaComment()); } From c302751c85c0066d1c2483b8944fe51305513fa9 Mon Sep 17 00:00:00 2001 From: yuqi Date: Tue, 4 Aug 2026 20:13:43 +0800 Subject: [PATCH 2/9] [#12342] fix(core): make managed namespace mutations atomic --- .../catalog/ManagedSchemaOperations.java | 5 +- .../relational/mapper/CatalogMetaMapper.java | 19 ++ .../mapper/CatalogMetaSQLProviderFactory.java | 12 + .../relational/mapper/MetalakeMetaMapper.java | 9 + .../MetalakeMetaSQLProviderFactory.java | 6 + .../relational/mapper/SchemaMetaMapper.java | 19 ++ .../mapper/SchemaMetaSQLProviderFactory.java | 12 + .../base/CatalogMetaBaseSQLProvider.java | 25 ++ .../base/MetalakeMetaBaseSQLProvider.java | 10 + .../base/SchemaMetaBaseSQLProvider.java | 24 ++ .../CatalogMetaPostgreSQLProvider.java | 15 + .../SchemaMetaPostgreSQLProvider.java | 14 + .../service/CatalogMetaService.java | 103 +++++-- .../service/MetalakeMetaService.java | 52 ++-- .../relational/service/SchemaMetaService.java | 286 ++++++++++-------- .../relational/utils/POConverters.java | 18 +- .../service/TestCatalogMetaService.java | 117 +++++++ .../service/TestMetalakeMetaService.java | 52 ++++ .../service/TestSchemaMetaService.java | 152 +++++++++- 19 files changed, 747 insertions(+), 203 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java b/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java index 164d4b53460..aca4eec619a 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java +++ b/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Map; import org.apache.gravitino.Entity; +import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.EntityStore; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; @@ -116,9 +117,11 @@ public Schema createSchema(NameIdentifier ident, String comment, Map catalogPOs); + @UpdateProvider( type = CatalogMetaSQLProviderFactory.class, method = "softDeleteCatalogMetasByMetalakeId") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java index 506c7874b9b..86467f6cef2 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java @@ -109,11 +109,23 @@ public static String updateCatalogMeta( return getProvider().updateCatalogMeta(newCatalogPO, oldCatalogPO); } + /** Returns SQL that advances a catalog OCC version conditionally. */ + public static String fenceCatalogMeta( + @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion) { + return getProvider().fenceCatalogMeta(catalogId, currentVersion); + } + public static String softDeleteCatalogMetasByCatalogId( @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion) { return getProvider().softDeleteCatalogMetasByCatalogId(catalogId, currentVersion); } + /** Returns SQL that soft-deletes catalogs using identifier-and-version pairs. */ + public static String softDeleteCatalogMetasWithVersion( + @Param("catalogMetas") List catalogPOs) { + return getProvider().softDeleteCatalogMetasWithVersion(catalogPOs); + } + public static String softDeleteCatalogMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { return getProvider().softDeleteCatalogMetasByMetalakeId(metalakeId); } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java index 33787a71a59..4b40bc751ea 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java @@ -70,6 +70,15 @@ Integer updateMetalakeMeta( @Param("newMetalakeMeta") MetalakePO newMetalakePO, @Param("oldMetalakeMeta") MetalakePO oldMetalakePO); + /** + * Advances the metalake version when the expected OCC version still matches. + * + * @return the number of updated rows + */ + @UpdateProvider(type = MetalakeMetaSQLProviderFactory.class, method = "fenceMetalakeMeta") + Integer fenceMetalakeMeta( + @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion); + @UpdateProvider( type = MetalakeMetaSQLProviderFactory.class, method = "softDeleteMetalakeMetaByMetalakeId") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java index 6d95b3a1e43..79a91f41f7e 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java @@ -88,6 +88,12 @@ public static String updateMetalakeMeta( return getProvider().updateMetalakeMeta(newMetalakePO, oldMetalakePO); } + /** Returns SQL that advances a metalake OCC version conditionally. */ + public static String fenceMetalakeMeta( + @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { + return getProvider().fenceMetalakeMeta(metalakeId, currentVersion); + } + public static String softDeleteMetalakeMetaByMetalakeId( @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { return getProvider().softDeleteMetalakeMetaByMetalakeId(metalakeId, currentVersion); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java index e8310e977c0..532a0b7d247 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java @@ -102,6 +102,15 @@ SchemaPO selectSchemaByFullQualifiedName( Integer updateSchemaMeta( @Param("newSchemaMeta") SchemaPO newSchemaPO, @Param("oldSchemaMeta") SchemaPO oldSchemaPO); + /** + * Advances the schema version when the expected OCC version still matches. + * + * @return the number of updated rows + */ + @UpdateProvider(type = SchemaMetaSQLProviderFactory.class, method = "fenceSchemaMeta") + Integer fenceSchemaMeta( + @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion); + @UpdateProvider( type = SchemaMetaSQLProviderFactory.class, method = "softDeleteSchemaMetasBySchemaIds") @@ -113,6 +122,16 @@ Integer updateSchemaMeta( Integer softDeleteSchemaMetaBySchemaIdAndVersion( @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion); + /** + * Soft-deletes schemas whose identifiers and OCC versions still match. + * + * @return the number of deleted rows + */ + @UpdateProvider( + type = SchemaMetaSQLProviderFactory.class, + method = "softDeleteSchemaMetasWithVersion") + Integer softDeleteSchemaMetasWithVersion(@Param("schemaMetas") List schemaPOs); + @UpdateProvider( type = SchemaMetaSQLProviderFactory.class, method = "softDeleteSchemaMetasByMetalakeId") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java index c65b23ad142..ba368c219fa 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java @@ -116,6 +116,12 @@ public static String updateSchemaMeta( return getProvider().updateSchemaMeta(newSchemaPO, oldSchemaPO); } + /** Returns SQL that advances a schema OCC version conditionally. */ + public static String fenceSchemaMeta( + @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion) { + return getProvider().fenceSchemaMeta(schemaId, currentVersion); + } + public static String softDeleteSchemaMetasBySchemaIds(@Param("schemaIds") List schemaIds) { return getProvider().softDeleteSchemaMetasBySchemaIds(schemaIds); } @@ -125,6 +131,12 @@ public static String softDeleteSchemaMetaBySchemaIdAndVersion( return getProvider().softDeleteSchemaMetaBySchemaIdAndVersion(schemaId, currentVersion); } + /** Returns SQL that soft-deletes schemas using identifier-and-version pairs. */ + public static String softDeleteSchemaMetasWithVersion( + @Param("schemaMetas") List schemaPOs) { + return getProvider().softDeleteSchemaMetasWithVersion(schemaPOs); + } + public static String softDeleteSchemaMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { return getProvider().softDeleteSchemaMetasByMetalakeId(metalakeId); } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java index 60934546d1d..bdaee81ac3b 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java @@ -210,6 +210,16 @@ public String updateCatalogMeta( + " AND deleted_at = 0"; } + /** Returns SQL that advances a catalog OCC version conditionally. */ + public String fenceCatalogMeta( + @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion) { + return "UPDATE " + + TABLE_NAME + + " SET last_version = current_version + 1, current_version = current_version + 1" + + " WHERE catalog_id = #{catalogId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; + } + public String softDeleteCatalogMetasByCatalogId( @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion) { return "UPDATE " @@ -220,6 +230,21 @@ public String softDeleteCatalogMetasByCatalogId( + " AND current_version = #{currentVersion} AND deleted_at = 0"; } + /** Returns SQL that soft-deletes catalogs using identifier-and-version pairs. */ + public String softDeleteCatalogMetasWithVersion( + @Param("catalogMetas") List catalogPOs) { + return ""; + } + public String softDeleteCatalogMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { return "UPDATE " + TABLE_NAME diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java index 4d0a920a7c0..5899b3d42b9 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java @@ -147,6 +147,16 @@ public String updateMetalakeMeta( + " AND deleted_at = 0"; } + /** Returns SQL that advances a metalake OCC version conditionally. */ + public String fenceMetalakeMeta( + @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { + return "UPDATE " + + TABLE_NAME + + " SET last_version = current_version + 1, current_version = current_version + 1" + + " WHERE metalake_id = #{metalakeId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; + } + public String softDeleteMetalakeMetaByMetalakeId( @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { return "UPDATE " diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java index 3bc2ec3555f..df7fd809586 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java @@ -277,6 +277,16 @@ public String updateSchemaMeta( + " AND deleted_at = 0"; } + /** Returns SQL that advances a schema OCC version conditionally. */ + public String fenceSchemaMeta( + @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion) { + return "UPDATE " + + TABLE_NAME + + " SET last_version = current_version + 1, current_version = current_version + 1" + + " WHERE schema_id = #{schemaId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; + } + public String softDeleteSchemaMetasBySchemaIds(@Param("schemaIds") List schemaIds) { return ""; + } + public String softDeleteSchemaMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { return "UPDATE " + TABLE_NAME diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java index 79f704162b8..462294d882c 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java @@ -20,6 +20,7 @@ import static org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper.TABLE_NAME; +import java.util.List; import org.apache.gravitino.storage.relational.mapper.provider.base.CatalogMetaBaseSQLProvider; import org.apache.gravitino.storage.relational.po.CatalogPO; import org.apache.ibatis.annotations.Param; @@ -34,6 +35,20 @@ public String softDeleteCatalogMetasByCatalogId(Long catalogId, Long currentVers + " AND current_version = #{currentVersion} AND deleted_at = 0"; } + /** {@inheritDoc} */ + @Override + public String softDeleteCatalogMetasWithVersion(List catalogPOs) { + return ""; + } + @Override public String softDeleteCatalogMetasByMetalakeId(Long metalakeId) { return "UPDATE " diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java index e2e6f9557e8..b451dd2b8de 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java @@ -125,6 +125,20 @@ public String softDeleteSchemaMetaBySchemaIdAndVersion(Long schemaId, Long curre + " AND current_version = #{currentVersion} AND deleted_at = 0"; } + /** {@inheritDoc} */ + @Override + public String softDeleteSchemaMetasWithVersion(List schemaPOs) { + return ""; + } + @Override public String softDeleteSchemaMetasByMetalakeId(Long metalakeId) { return "UPDATE " diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java index c4791c8c395..a8886c56c27 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java @@ -36,7 +36,6 @@ import org.apache.gravitino.exceptions.NonEmptyEntityException; import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.CatalogEntity; -import org.apache.gravitino.meta.SchemaEntity; import org.apache.gravitino.metrics.Monitored; import org.apache.gravitino.storage.relational.helper.CatalogIds; import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; @@ -45,6 +44,7 @@ import org.apache.gravitino.storage.relational.mapper.FilesetVersionMapper; import org.apache.gravitino.storage.relational.mapper.FunctionMetaMapper; import org.apache.gravitino.storage.relational.mapper.FunctionVersionMetaMapper; +import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper; import org.apache.gravitino.storage.relational.mapper.ModelMetaMapper; import org.apache.gravitino.storage.relational.mapper.ModelVersionAliasRelMapper; import org.apache.gravitino.storage.relational.mapper.ModelVersionMetaMapper; @@ -59,6 +59,8 @@ import org.apache.gravitino.storage.relational.mapper.TopicMetaMapper; import org.apache.gravitino.storage.relational.mapper.ViewMetaMapper; import org.apache.gravitino.storage.relational.po.CatalogPO; +import org.apache.gravitino.storage.relational.po.MetalakePO; +import org.apache.gravitino.storage.relational.po.SchemaPO; import org.apache.gravitino.storage.relational.po.cache.OperateType; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; @@ -182,20 +184,32 @@ public void insertCatalog(CatalogEntity catalogEntity, boolean overwrite) throws try { NameIdentifierUtil.checkCatalog(catalogEntity.nameIdentifier()); - String metalake = NameIdentifierUtil.getMetalake(catalogEntity.nameIdentifier()); - Long metalakeId = - EntityIdService.getEntityId(NameIdentifier.of(metalake), Entity.EntityType.METALAKE); - - SessionUtils.doWithCommit( - CatalogMetaMapper.class, - mapper -> { - CatalogPO po = POConverters.initializeCatalogPOWithVersion(catalogEntity, metalakeId); - if (overwrite) { - mapper.insertCatalogMetaOnDuplicateKeyUpdate(po); - } else { - mapper.insertCatalogMeta(po); - } - }); + String metalakeName = NameIdentifierUtil.getMetalake(catalogEntity.nameIdentifier()); + MetalakePO metalakePO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalakeName)); + if (metalakePO == null) { + throw new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.METALAKE.name().toLowerCase(), + metalakeName); + } + + SessionUtils.doMultipleWithCommit( + () -> fenceMetalakeForCatalogCreate(metalakePO), + () -> + SessionUtils.doWithoutCommit( + CatalogMetaMapper.class, + mapper -> { + CatalogPO po = + POConverters.initializeCatalogPOWithVersion( + catalogEntity, metalakePO.getMetalakeId()); + if (overwrite) { + mapper.insertCatalogMetaOnDuplicateKeyUpdate(po); + } else { + mapper.insertCatalogMeta(po); + } + })); } catch (RuntimeException re) { ExceptionUtils.checkSQLException( re, Entity.EntityType.CATALOG, catalogEntity.nameIdentifier().toString()); @@ -278,11 +292,10 @@ public boolean deleteCatalog(NameIdentifier identifier, boolean cascade) { if (cascade) { SessionUtils.doMultipleWithCommit( - () -> deleteCatalogWithVersion(identifier, catalogId, currentVersion), - () -> - SessionUtils.doWithoutCommit( - SchemaMetaMapper.class, - mapper -> mapper.softDeleteSchemaMetasByCatalogId(catalogId)), + () -> { + deleteCatalogWithVersion(identifier, catalogId, currentVersion); + deleteSchemasWithVersions(identifier, catalogId); + }, () -> SessionUtils.doWithoutCommit( TableMetaMapper.class, @@ -359,16 +372,17 @@ public boolean deleteCatalog(NameIdentifier identifier, boolean cascade) { OperateType.DROP)); }); } else { - List schemaEntities = - SchemaMetaService.getInstance() - .listSchemasByNamespace( - NamespaceUtil.ofSchema(identifier.namespace().level(0), catalogName)); - if (!schemaEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", identifier); - } SessionUtils.doMultipleWithCommit( - () -> deleteCatalogWithVersion(identifier, catalogId, currentVersion), + () -> { + deleteCatalogWithVersion(identifier, catalogId, currentVersion); + List schemaPOs = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.listSchemaPOsByCatalogId(catalogId)); + if (!schemaPOs.isEmpty()) { + throw new NonEmptyEntityException( + "Entity %s has sub-entities, you should remove sub-entities first", identifier); + } + }, () -> SessionUtils.doWithoutCommit( OwnerMetaMapper.class, @@ -423,6 +437,37 @@ private void deleteCatalogWithVersion( } } + private void fenceMetalakeForCatalogCreate(MetalakePO metalakePO) { + int fenced = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, + mapper -> + mapper.fenceMetalakeMeta( + metalakePO.getMetalakeId(), metalakePO.getCurrentVersion())); + if (fenced == 0) { + throw new OptimisticLockException( + "The parent metalake %s was modified concurrently; retry the operation", + metalakePO.getMetalakeName()); + } + } + + private void deleteSchemasWithVersions(NameIdentifier catalogIdentifier, Long catalogId) { + List schemaPOs = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.listSchemaPOsByCatalogId(catalogId)); + if (schemaPOs.isEmpty()) { + return; + } + int deleted = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.softDeleteSchemaMetasWithVersion(schemaPOs)); + if (deleted != schemaPOs.size()) { + throw new OptimisticLockException( + "A schema under catalog %s was modified concurrently; retry the operation", + catalogIdentifier); + } + } + private OptimisticLockException optimisticLockException(NameIdentifier identifier) { return new OptimisticLockException( "The catalog %s was modified concurrently; retry the operation", identifier); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java index 968456844e1..824279fa143 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java @@ -35,7 +35,6 @@ import org.apache.gravitino.exceptions.NonEmptyEntityException; import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.BaseMetalake; -import org.apache.gravitino.meta.CatalogEntity; import org.apache.gravitino.metrics.Monitored; import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper; @@ -66,13 +65,13 @@ import org.apache.gravitino.storage.relational.mapper.UserMetaMapper; import org.apache.gravitino.storage.relational.mapper.UserRoleRelMapper; import org.apache.gravitino.storage.relational.mapper.ViewMetaMapper; +import org.apache.gravitino.storage.relational.po.CatalogPO; import org.apache.gravitino.storage.relational.po.MetalakePO; import org.apache.gravitino.storage.relational.po.cache.OperateType; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; import org.apache.gravitino.utils.NameIdentifierUtil; -import org.apache.gravitino.utils.NamespaceUtil; /** * The service class for metalake metadata. It provides the basic database operations for metalake. @@ -233,11 +232,10 @@ public boolean deleteMetalake(NameIdentifier ident, boolean cascade) { if (metalakeId != null) { if (cascade) { SessionUtils.doMultipleWithCommit( - () -> deleteMetalakeWithVersion(ident, metalakeId, currentVersion), - () -> - SessionUtils.doWithoutCommit( - CatalogMetaMapper.class, - mapper -> mapper.softDeleteCatalogMetasByMetalakeId(metalakeId)), + () -> { + deleteMetalakeWithVersion(ident, metalakeId, currentVersion); + deleteCatalogsWithVersions(ident, metalakeId); + }, () -> SessionUtils.doWithoutCommit( SchemaMetaMapper.class, @@ -353,15 +351,18 @@ public boolean deleteMetalake(NameIdentifier ident, boolean cascade) { OperateType.DROP)); }); } else { - List catalogEntities = - CatalogMetaService.getInstance() - .listCatalogsByNamespace(NamespaceUtil.ofCatalog(ident.name())); - if (!catalogEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", ident); - } SessionUtils.doMultipleWithCommit( - () -> deleteMetalakeWithVersion(ident, metalakeId, currentVersion), + () -> { + deleteMetalakeWithVersion(ident, metalakeId, currentVersion); + List catalogPOs = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.listCatalogPOsByMetalakeId(metalakeId)); + if (!catalogPOs.isEmpty()) { + throw new NonEmptyEntityException( + "Entity %s has sub-entities, you should remove sub-entities first", ident); + } + }, () -> SessionUtils.doWithoutCommit( UserRoleRelMapper.class, @@ -425,8 +426,7 @@ public boolean deleteMetalake(NameIdentifier ident, boolean cascade) { return true; } - private void deleteMetalakeWithVersion( - NameIdentifier identifier, Long metalakeId, Long currentVersion) { + void deleteMetalakeWithVersion(NameIdentifier identifier, Long metalakeId, Long currentVersion) { int deleted = SessionUtils.getWithoutCommit( MetalakeMetaMapper.class, @@ -436,6 +436,24 @@ private void deleteMetalakeWithVersion( } } + private void deleteCatalogsWithVersions(NameIdentifier metalakeIdentifier, Long metalakeId) { + List catalogPOs = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.listCatalogPOsByMetalakeId(metalakeId)); + if (catalogPOs.isEmpty()) { + return; + } + int deleted = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.softDeleteCatalogMetasWithVersion(catalogPOs)); + if (deleted != catalogPOs.size()) { + throw new OptimisticLockException( + "A catalog under metalake %s was modified concurrently; retry the operation", + metalakeIdentifier); + } + } + private OptimisticLockException optimisticLockException(NameIdentifier identifier) { return new OptimisticLockException( "The metalake %s was modified concurrently; retry the operation", identifier); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java index aa17ec6f4bc..f69ee3e96a5 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java @@ -26,9 +26,10 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -41,15 +42,11 @@ import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NonEmptyEntityException; import org.apache.gravitino.exceptions.OptimisticLockException; -import org.apache.gravitino.meta.FilesetEntity; -import org.apache.gravitino.meta.ModelEntity; -import org.apache.gravitino.meta.NamespacedEntityId; import org.apache.gravitino.meta.SchemaEntity; -import org.apache.gravitino.meta.TableEntity; -import org.apache.gravitino.meta.TopicEntity; import org.apache.gravitino.metrics.Monitored; import org.apache.gravitino.storage.IdGenerator; import org.apache.gravitino.storage.relational.helper.SchemaIds; +import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper; import org.apache.gravitino.storage.relational.mapper.FilesetMetaMapper; import org.apache.gravitino.storage.relational.mapper.FilesetVersionMapper; @@ -68,6 +65,7 @@ import org.apache.gravitino.storage.relational.mapper.TagMetadataObjectRelMapper; import org.apache.gravitino.storage.relational.mapper.TopicMetaMapper; import org.apache.gravitino.storage.relational.mapper.ViewMetaMapper; +import org.apache.gravitino.storage.relational.po.CatalogPO; import org.apache.gravitino.storage.relational.po.SchemaPO; import org.apache.gravitino.storage.relational.po.cache.OperateType; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; @@ -146,6 +144,10 @@ public void insertSchema(SchemaEntity schemaEntity, boolean overwrite) throws IO // rewriter to translate each PO's name to storage form before SQL execution. String logicalSep = HierarchicalSchemaUtil.schemaSeparator(); String schemaName = schemaEntity.name(); + String metalakeName = schemaEntity.namespace().level(0); + String catalogName = schemaEntity.namespace().level(1); + CatalogPO catalogPO = + CatalogMetaService.getInstance().getCatalogPOByName(metalakeName, catalogName); List rowsToInsert = new ArrayList<>(); if (schemaName == null || !schemaName.contains(logicalSep)) { rowsToInsert.add(schemaEntity); @@ -168,39 +170,46 @@ public void insertSchema(SchemaEntity schemaEntity, boolean overwrite) throws IO rowsToInsert.add(schemaEntity); } - SessionUtils.doWithCommit( - SchemaMetaMapper.class, - mapper -> { - int n = rowsToInsert.size(); - List missingAncestorPOs = new ArrayList<>(); - if (n > 1) { - SchemaEntity firstAncestor = rowsToInsert.get(0); - Namespace ancestorNs = firstAncestor.namespace(); - List ancestorNames = - rowsToInsert.subList(0, n - 1).stream() - .map(SchemaEntity::name) - .collect(Collectors.toList()); - Set existingLogicalNames = - ops.listPOs(mapper, ancestorNs, ancestorNames).stream() - .map(SchemaPO::getSchemaName) - .collect(Collectors.toSet()); - for (SchemaEntity row : rowsToInsert.subList(0, n - 1)) { - if (existingLogicalNames.contains(row.name())) { - continue; - } - SchemaPO.Builder builder = SchemaPO.builder(); - fillSchemaPOBuilderParentEntityId(builder, row.namespace()); - missingAncestorPOs.add(POConverters.initializeSchemaPOWithVersion(row, builder)); - } - } - SchemaEntity leafRow = rowsToInsert.get(n - 1); - SchemaPO.Builder leafBuilder = SchemaPO.builder(); - fillSchemaPOBuilderParentEntityId(leafBuilder, leafRow.namespace()); - SchemaPO leafPO = POConverters.initializeSchemaPOWithVersion(leafRow, leafBuilder); - List schemaPosToInsert = new ArrayList<>(missingAncestorPOs); - schemaPosToInsert.add(leafPO); - ops.batchInsertPOs(mapper, schemaPosToInsert, overwrite); - }); + SessionUtils.doMultipleWithCommit( + () -> fenceCatalogForSchemaCreate(catalogPO), + () -> + SessionUtils.doWithoutCommit( + SchemaMetaMapper.class, + mapper -> { + int n = rowsToInsert.size(); + List missingAncestorPOs = new ArrayList<>(); + if (n > 1) { + SchemaEntity firstAncestor = rowsToInsert.get(0); + Namespace ancestorNs = firstAncestor.namespace(); + List ancestorNames = + rowsToInsert.subList(0, n - 1).stream() + .map(SchemaEntity::name) + .collect(Collectors.toList()); + Map existingAncestors = + ops.listPOs(mapper, ancestorNs, ancestorNames).stream() + .collect( + Collectors.toMap(SchemaPO::getSchemaName, Function.identity())); + for (SchemaEntity row : rowsToInsert.subList(0, n - 1)) { + SchemaPO existingAncestor = existingAncestors.get(row.name()); + if (existingAncestor != null) { + fenceSchemaAncestor( + mapper, existingAncestor, schemaEntity.nameIdentifier()); + continue; + } + SchemaPO.Builder builder = newSchemaPOBuilder(catalogPO); + missingAncestorPOs.add( + POConverters.initializeSchemaPOWithVersion(row, builder)); + } + } + if (!missingAncestorPOs.isEmpty()) { + ops.batchInsertPOs(mapper, missingAncestorPOs, false); + } + SchemaEntity leafRow = rowsToInsert.get(n - 1); + SchemaPO leafPO = + POConverters.initializeSchemaPOWithVersion( + leafRow, newSchemaPOBuilder(catalogPO)); + ops.batchInsertPOs(mapper, Collections.singletonList(leafPO), overwrite); + })); } catch (RuntimeException re) { ExceptionUtils.checkSQLException( re, Entity.EntityType.SCHEMA, schemaEntity.nameIdentifier().toString()); @@ -280,87 +289,81 @@ public boolean deleteSchema(NameIdentifier identifier, boolean cascade) { NameIdentifierUtil.ofSchema(metalakeName, catalogName, schemaName).toString(); if (cascade) { - // For HierarchicalSchema, deleting `A:B` must also cascade into all descendant schemas - // such as `A:B:C`, `A:B:C:D`, etc. Collect the descendant schema ids up-front and run a - // single batch UPDATE per child table so the total SQL cost stays bounded regardless of - // how many descendants exist. - List schemaIds = listSchemaIdsForCascade(schemaPO); - if (schemaIds.isEmpty()) { - return false; - } - List descendantSchemaIds = - schemaIds.stream().filter(id -> !id.equals(schemaId)).collect(Collectors.toList()); + AtomicReference> schemaIds = new AtomicReference<>(); SessionUtils.doMultipleWithCommit( - () -> deleteSchemaWithVersion(identifier, schemaId, schemaPO.getCurrentVersion()), () -> { - if (!descendantSchemaIds.isEmpty()) { - SessionUtils.doWithoutCommit( - SchemaMetaMapper.class, - mapper -> mapper.softDeleteSchemaMetasBySchemaIds(descendantSchemaIds)); - } + deleteSchemaWithVersion(identifier, schemaId, schemaPO.getCurrentVersion()); + List descendants = listDescendantSchemaPOs(schemaPO); + deleteDescendantSchemasWithVersions(identifier, descendants); + List ids = new ArrayList<>(descendants.size() + 1); + ids.add(schemaId); + descendants.stream().map(SchemaPO::getSchemaId).forEach(ids::add); + schemaIds.set(ids); }, () -> SessionUtils.doWithoutCommit( TableMetaMapper.class, - mapper -> mapper.softDeleteTableMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteTableMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( TableColumnMapper.class, - mapper -> mapper.softDeleteColumnsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteColumnsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( FilesetMetaMapper.class, - mapper -> mapper.softDeleteFilesetMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteFilesetMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( FilesetVersionMapper.class, - mapper -> mapper.softDeleteFilesetVersionsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteFilesetVersionsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( TopicMetaMapper.class, - mapper -> mapper.softDeleteTopicMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteTopicMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( FunctionMetaMapper.class, - mapper -> mapper.softDeleteFunctionMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteFunctionMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( FunctionVersionMetaMapper.class, - mapper -> mapper.softDeleteFunctionVersionMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteFunctionVersionMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( - OwnerMetaMapper.class, mapper -> mapper.softDeleteOwnerRelBySchemaIds(schemaIds)), + OwnerMetaMapper.class, + mapper -> mapper.softDeleteOwnerRelBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( SecurableObjectMapper.class, - mapper -> mapper.softDeleteObjectRelsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteObjectRelsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( TagMetadataObjectRelMapper.class, - mapper -> mapper.softDeleteTagMetadataObjectRelsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteTagMetadataObjectRelsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( PolicyMetadataObjectRelMapper.class, - mapper -> mapper.softDeletePolicyMetadataObjectRelsBySchemaIds(schemaIds)), + mapper -> mapper.softDeletePolicyMetadataObjectRelsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( ModelVersionAliasRelMapper.class, - mapper -> mapper.softDeleteModelVersionAliasRelsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteModelVersionAliasRelsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( ModelVersionMetaMapper.class, - mapper -> mapper.softDeleteModelVersionMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteModelVersionMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( ModelMetaMapper.class, - mapper -> mapper.softDeleteModelMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteModelMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( StatisticMetaMapper.class, - mapper -> mapper.softDeleteStatisticsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteStatisticsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( - ViewMetaMapper.class, mapper -> mapper.softDeleteViewMetasBySchemaIds(schemaIds)), + ViewMetaMapper.class, + mapper -> mapper.softDeleteViewMetasBySchemaIds(schemaIds.get())), () -> { SessionUtils.doWithoutCommit( EntityChangeLogMapper.class, @@ -372,54 +375,11 @@ public boolean deleteSchema(NameIdentifier identifier, boolean cascade) { OperateType.DROP)); }); } else { - List tableEntities = - TableMetaService.getInstance() - .listTablesByNamespace( - NamespaceUtil.ofTable( - identifier.namespace().level(0), - identifier.namespace().level(1), - schemaName)); - if (!tableEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", identifier); - } - List filesetEntities = - FilesetMetaService.getInstance() - .listFilesetsByNamespace( - NamespaceUtil.ofFileset( - identifier.namespace().level(0), - identifier.namespace().level(1), - schemaName)); - if (!filesetEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", identifier); - } - List modelEntities = - ModelMetaService.getInstance() - .listModelsByNamespace( - NamespaceUtil.ofModel( - identifier.namespace().level(0), - identifier.namespace().level(1), - schemaName)); - if (!modelEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", identifier); - } - - List topicEntities = - TopicMetaService.getInstance() - .listTopicsByNamespace( - NamespaceUtil.ofTopic( - identifier.namespace().level(0), - identifier.namespace().level(1), - schemaName)); - if (!topicEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", identifier); - } - SessionUtils.doMultipleWithCommit( - () -> deleteSchemaWithVersion(identifier, schemaId, schemaPO.getCurrentVersion()), + () -> { + deleteSchemaWithVersion(identifier, schemaId, schemaPO.getCurrentVersion()); + checkSchemaIsEmpty(identifier, schemaPO); + }, () -> SessionUtils.doWithoutCommit( OwnerMetaMapper.class, @@ -511,13 +471,78 @@ private List listSchemaPOs(Namespace namespace) { mapper -> POStorageReadRouting.listPOs(mapper, namespace, ops, Entity.EntityType.SCHEMA)); } + private void fenceCatalogForSchemaCreate(CatalogPO catalogPO) { + int fenced = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> + mapper.fenceCatalogMeta(catalogPO.getCatalogId(), catalogPO.getCurrentVersion())); + if (fenced == 0) { + throw new OptimisticLockException( + "The parent catalog %s was modified concurrently; retry the operation", + catalogPO.getCatalogName()); + } + } + + private void fenceSchemaAncestor( + SchemaMetaMapper mapper, SchemaPO ancestor, NameIdentifier schemaIdentifier) { + int fenced = mapper.fenceSchemaMeta(ancestor.getSchemaId(), ancestor.getCurrentVersion()); + if (fenced == 0) { + throw new OptimisticLockException( + "An ancestor of schema %s was modified concurrently; retry the operation", + schemaIdentifier); + } + } + + private void deleteDescendantSchemasWithVersions( + NameIdentifier schemaIdentifier, List descendants) { + if (descendants.isEmpty()) { + return; + } + int deleted = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.softDeleteSchemaMetasWithVersion(descendants)); + if (deleted != descendants.size()) { + throw new OptimisticLockException( + "A descendant of schema %s was modified concurrently; retry the operation", + schemaIdentifier); + } + } + + private void checkSchemaIsEmpty(NameIdentifier identifier, SchemaPO schemaPO) { + boolean hasDescendantSchemas = !listDescendantSchemaPOs(schemaPO).isEmpty(); + boolean hasTables = + !SessionUtils.getWithoutCommit( + TableMetaMapper.class, + mapper -> mapper.listTablePOsBySchemaId(schemaPO.getSchemaId())) + .isEmpty(); + boolean hasFilesets = + !SessionUtils.getWithoutCommit( + FilesetMetaMapper.class, + mapper -> mapper.listFilesetPOsBySchemaId(schemaPO.getSchemaId())) + .isEmpty(); + boolean hasModels = + !SessionUtils.getWithoutCommit( + ModelMetaMapper.class, + mapper -> mapper.listModelPOsBySchemaId(schemaPO.getSchemaId())) + .isEmpty(); + boolean hasTopics = + !SessionUtils.getWithoutCommit( + TopicMetaMapper.class, + mapper -> mapper.listTopicPOsBySchemaId(schemaPO.getSchemaId())) + .isEmpty(); + if (hasDescendantSchemas || hasTables || hasFilesets || hasModels || hasTopics) { + throw new NonEmptyEntityException( + "Entity %s has sub-entities, you should remove sub-entities first", identifier); + } + } + /** - * Collects the schema ids that participate in a cascade delete: the target schema itself plus - * every HierarchicalSchema descendant. The {@link SchemaPO} arrives in logical form (e.g. {@code - * A:B}); {@link HierarchicalConversionPOStorageOps} translates to storage form before running the - * SQL prefix match, so this method only deals in logical names. + * Collects every HierarchicalSchema descendant of the target schema. The {@link SchemaPO} arrives + * in logical form (e.g. {@code A:B}); {@link HierarchicalConversionPOStorageOps} translates to + * storage form before running the SQL prefix match. */ - private List listSchemaIdsForCascade(SchemaPO schemaPO) { + private List listDescendantSchemaPOs(SchemaPO schemaPO) { List matched = SessionUtils.getWithoutCommit( SchemaMetaMapper.class, @@ -526,16 +551,15 @@ private List listSchemaIdsForCascade(SchemaPO schemaPO) { if (matched == null || matched.isEmpty()) { return Collections.emptyList(); } - return matched.stream().map(SchemaPO::getSchemaId).collect(Collectors.toList()); + return matched.stream() + .filter(po -> !po.getSchemaId().equals(schemaPO.getSchemaId())) + .collect(Collectors.toList()); } - private void fillSchemaPOBuilderParentEntityId(SchemaPO.Builder builder, Namespace namespace) { - NamespaceUtil.checkSchema(namespace); - NamespacedEntityId namespacedEntityId = - EntityIdService.getEntityIds( - NameIdentifier.of(namespace.levels()), Entity.EntityType.CATALOG); - builder.withMetalakeId(namespacedEntityId.namespaceIds()[0]); - builder.withCatalogId(namespacedEntityId.entityId()); + private SchemaPO.Builder newSchemaPOBuilder(CatalogPO catalogPO) { + return SchemaPO.builder() + .withMetalakeId(catalogPO.getMetalakeId()) + .withCatalogId(catalogPO.getCatalogId()); } @Monitored( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java b/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java index 16a274ce05c..dd00e96a00b 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java @@ -136,9 +136,9 @@ public static MetalakePO initializeMetalakePOWithVersion(BaseMetalake baseMetala */ public static MetalakePO updateMetalakePOWithVersion( MetalakePO oldMetalakePO, BaseMetalake newMetalake) { - Long lastVersion = oldMetalakePO.getLastVersion(); - // Will set the version to the last version + 1 when having some fields need be multiple version - Long nextVersion = lastVersion + 1; + // Every metadata update advances the OCC token. Both version columns stay aligned because + // metalakes do not retain independently addressable historical versions. + Long nextVersion = oldMetalakePO.getCurrentVersion() + 1; try { return MetalakePO.builder() .withMetalakeId(newMetalake.id()) @@ -233,9 +233,9 @@ public static CatalogPO initializeCatalogPOWithVersion( */ public static CatalogPO updateCatalogPOWithVersion( CatalogPO oldCatalogPO, CatalogEntity newCatalog, Long metalakeId) { - Long lastVersion = oldCatalogPO.getLastVersion(); - // Will set the version to the last version + 1 when having some fields need be multiple version - Long nextVersion = lastVersion + 1; + // Every metadata update advances the OCC token. Both version columns stay aligned because + // catalogs do not retain independently addressable historical versions. + Long nextVersion = oldCatalogPO.getCurrentVersion() + 1; try { return CatalogPO.builder() .withCatalogId(newCatalog.id()) @@ -329,9 +329,9 @@ public static SchemaPO initializeSchemaPOWithVersion( * @return SchemaPO object with updated version */ public static SchemaPO updateSchemaPOWithVersion(SchemaPO oldSchemaPO, SchemaEntity newSchema) { - Long lastVersion = oldSchemaPO.getLastVersion(); - // Will set the version to the last version + 1 when having some fields need be multiple version - Long nextVersion = lastVersion + 1; + // Every metadata update advances the OCC token. Both version columns stay aligned because + // schemas do not retain independently addressable historical versions. + Long nextVersion = oldSchemaPO.getCurrentVersion() + 1; try { return SchemaPO.builder() .withSchemaId(oldSchemaPO.getSchemaId()) diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java index c64f07bd1fc..a6468fe5f32 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java @@ -35,6 +35,8 @@ import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; +import org.apache.gravitino.exceptions.NonEmptyEntityException; +import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.CatalogEntity; import org.apache.gravitino.meta.ColumnEntity; @@ -50,7 +52,9 @@ import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.TestJDBCBackend; import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; +import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper; import org.apache.gravitino.storage.relational.po.CatalogPO; +import org.apache.gravitino.storage.relational.po.MetalakePO; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; @@ -90,6 +94,40 @@ public void testInsertAlreadyExistsException() throws IOException { assertThrows(EntityAlreadyExistsException.class, () -> backend.insert(catalogCopy, false)); } + @TestTemplate + public void testInsertCatalogFencesMetalakeAndRollsBackFenceOnFailure() throws IOException { + MetalakePO beforeInsert = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalakeName)); + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "catalog_fence", + auditInfo); + backend.insert(catalog, false); + + MetalakePO afterInsert = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalakeName)); + assertEquals(beforeInsert.getCurrentVersion() + 1, afterInsert.getCurrentVersion()); + assertEquals(afterInsert.getCurrentVersion(), afterInsert.getLastVersion()); + + CatalogEntity duplicate = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + catalog.name(), + auditInfo); + assertThrows(EntityAlreadyExistsException.class, () -> backend.insert(duplicate, false)); + + MetalakePO afterFailure = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalakeName)); + assertEquals(afterInsert.getCurrentVersion(), afterFailure.getCurrentVersion()); + assertEquals(afterInsert.getLastVersion(), afterFailure.getLastVersion()); + } + @TestTemplate public void testUpdateAlreadyExistsException() throws IOException { CatalogEntity catalog = @@ -200,6 +238,72 @@ public void testAlterAndDeleteUseCurrentVersion() throws IOException { assertEquals(1, deleted); } + @TestTemplate + public void testAlterReportsOptimisticLockConflict() throws IOException { + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "catalog_alter_conflict", + auditInfo); + backend.insert(catalog, false); + + assertThrows( + OptimisticLockException.class, + () -> + CatalogMetaService.getInstance() + .updateCatalog( + catalog.nameIdentifier(), + entity -> { + CatalogEntity current = (CatalogEntity) entity; + CatalogPO currentPO = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.selectCatalogMetaById(current.id())); + CatalogEntity competingUpdate = + copyCatalogWithComment(current, "competing update"); + CatalogPO competingPO = + POConverters.updateCatalogPOWithVersion( + currentPO, competingUpdate, currentPO.getMetalakeId()); + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, + mapper -> mapper.updateCatalogMeta(competingPO, currentPO)); + return copyCatalogWithComment(current, "requested update"); + })); + } + + @TestTemplate + public void testNonCascadeDeleteRollsBackCatalogFence() throws IOException { + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "catalog_non_empty", + auditInfo); + backend.insert(catalog, false); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalog.name()), + "schema", + auditInfo); + backend.insert(schema, false); + CatalogPO beforeDelete = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + + assertThrows( + NonEmptyEntityException.class, + () -> CatalogMetaService.getInstance().deleteCatalog(catalog.nameIdentifier(), false)); + + CatalogPO afterDelete = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + assertEquals(beforeDelete.getCurrentVersion(), afterDelete.getCurrentVersion()); + assertTrue(backend.exists(catalog.nameIdentifier(), Entity.EntityType.CATALOG)); + assertTrue(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + } + @TestTemplate public void testMetaLifeCycleFromCreationToDeletion() throws IOException { CatalogEntity catalog = @@ -354,6 +458,19 @@ public void testDeleteCatalogCascadeRemovesTagRelations() throws IOException { assertEquals(0, countActiveTagRelForMetadataObject(function.id(), "FUNCTION")); } + private CatalogEntity copyCatalogWithComment(CatalogEntity catalog, String comment) { + return CatalogEntity.builder() + .withId(catalog.id()) + .withName(catalog.name()) + .withNamespace(catalog.namespace()) + .withType(catalog.getType()) + .withProvider(catalog.getProvider()) + .withComment(comment) + .withProperties(catalog.getProperties()) + .withAuditInfo(auditInfo) + .build(); + } + private void associateTag(TagEntity tag, NameIdentifier ident, Entity.EntityType type) throws IOException { TagMetaService.getInstance() diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java index 618263479fb..d72650e8854 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java @@ -27,6 +27,7 @@ import java.util.List; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; +import org.apache.gravitino.exceptions.NonEmptyEntityException; import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.BaseMetalake; import org.apache.gravitino.meta.SchemaVersion; @@ -180,6 +181,57 @@ public void testAlterReportsOptimisticLockConflict() throws IOException { })); } + @TestTemplate + public void testDeleteReportsOptimisticLockConflict() throws IOException { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + MetalakePO stalePO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalake.name())); + BaseMetalake competingUpdate = + BaseMetalake.builder() + .withId(metalake.id()) + .withName(metalake.name()) + .withAuditInfo(metalake.auditInfo()) + .withComment("competing update") + .withProperties(metalake.properties()) + .withVersion(metalake.getVersion()) + .build(); + MetalakePO competingPO = POConverters.updateMetalakePOWithVersion(stalePO, competingUpdate); + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, mapper -> mapper.updateMetalakeMeta(competingPO, stalePO)); + + assertThrows( + OptimisticLockException.class, + () -> + SessionUtils.doMultipleWithCommit( + () -> + MetalakeMetaService.getInstance() + .deleteMetalakeWithVersion( + metalake.nameIdentifier(), + metalake.id(), + stalePO.getCurrentVersion()))); + assertTrue(backend.exists(metalake.nameIdentifier(), Entity.EntityType.METALAKE)); + } + + @TestTemplate + public void testNonCascadeDeleteRollsBackMetalakeFence() throws IOException { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + createAndInsertCatalog(METALAKE_NAME, "catalog"); + MetalakePO beforeDelete = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalake.name())); + + assertThrows( + NonEmptyEntityException.class, + () -> MetalakeMetaService.getInstance().deleteMetalake(metalake.nameIdentifier(), false)); + + MetalakePO afterDelete = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalake.name())); + Assertions.assertEquals(beforeDelete.getCurrentVersion(), afterDelete.getCurrentVersion()); + assertTrue(backend.exists(metalake.nameIdentifier(), Entity.EntityType.METALAKE)); + } + @TestTemplate public void testMetaLifeCycleFromCreationToDeletion() throws IOException { // meta data creation diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java index e47507a3245..9e8a84ca566 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java @@ -37,6 +37,8 @@ import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.NonEmptyEntityException; +import org.apache.gravitino.exceptions.OptimisticLockException; +import org.apache.gravitino.meta.CatalogEntity; import org.apache.gravitino.meta.ColumnEntity; import org.apache.gravitino.meta.FilesetEntity; import org.apache.gravitino.meta.FunctionEntity; @@ -49,7 +51,9 @@ import org.apache.gravitino.rel.types.Types; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.TestJDBCBackend; +import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; import org.apache.gravitino.storage.relational.mapper.SchemaMetaMapper; +import org.apache.gravitino.storage.relational.po.CatalogPO; import org.apache.gravitino.storage.relational.po.SchemaPO; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; import org.apache.gravitino.storage.relational.utils.POConverters; @@ -85,6 +89,42 @@ public void testInsertAlreadyExistsException() throws IOException { assertThrows(EntityAlreadyExistsException.class, () -> backend.insert(schemaCopy, false)); } + @TestTemplate + public void testInsertSchemaFencesCatalogAndRollsBackFenceOnFailure() throws IOException { + createAndInsertMakeLake(metalakeName); + CatalogEntity catalog = createAndInsertCatalog(metalakeName, catalogName); + CatalogPO beforeInsert = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "schema_fence", + AUDIT_INFO); + backend.insert(schema, false); + + CatalogPO afterInsert = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + Assertions.assertEquals(beforeInsert.getCurrentVersion() + 1, afterInsert.getCurrentVersion()); + Assertions.assertEquals(afterInsert.getCurrentVersion(), afterInsert.getLastVersion()); + + SchemaEntity duplicate = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + schema.name(), + AUDIT_INFO); + assertThrows(EntityAlreadyExistsException.class, () -> backend.insert(duplicate, false)); + + CatalogPO afterFailure = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + Assertions.assertEquals(afterInsert.getCurrentVersion(), afterFailure.getCurrentVersion()); + Assertions.assertEquals(afterInsert.getLastVersion(), afterFailure.getLastVersion()); + } + @TestTemplate public void testUpdateAlreadyExistsException() throws IOException { createAndInsertMakeLake(metalakeName); @@ -199,6 +239,41 @@ public void testAlterAndDeleteUseCurrentVersion() throws IOException { Assertions.assertEquals(1, deleted); } + @TestTemplate + public void testAlterReportsOptimisticLockConflict() throws IOException { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "schema_alter_conflict", + AUDIT_INFO); + backend.insert(schema, false); + + assertThrows( + OptimisticLockException.class, + () -> + SchemaMetaService.getInstance() + .updateSchema( + schema.nameIdentifier(), + entity -> { + SchemaEntity current = (SchemaEntity) entity; + SchemaPO currentPO = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> mapper.selectSchemaMetaById(current.id())); + SchemaEntity competingUpdate = + copySchemaWithComment(current, "competing update"); + SchemaPO competingPO = + POConverters.updateSchemaPOWithVersion(currentPO, competingUpdate); + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, + mapper -> mapper.updateSchemaMeta(competingPO, currentPO)); + return copySchemaWithComment(current, "requested update"); + })); + } + @TestTemplate public void testMetaLifeCycleFromCreationToDeletion() throws IOException { createAndInsertMakeLake(metalakeName); @@ -269,12 +344,22 @@ public void testDeleteSchemaNonCascadingFailsWhenTopicExists() throws IOExceptio topicName, AUDIT_INFO); topicMetaService.insertTopic(topic, false); + SchemaPO beforeDelete = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); Assertions.assertThrows( NonEmptyEntityException.class, () -> schemaMetaService.deleteSchema(schema.nameIdentifier(), false), "Non-cascading delete must fail when dependent topics exist."); + SchemaPO afterDelete = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); + Assertions.assertEquals(beforeDelete.getCurrentVersion(), afterDelete.getCurrentVersion()); + assertTrue(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + assertTrue(backend.exists(topic.nameIdentifier(), Entity.EntityType.TOPIC)); + topicMetaService.deleteTopic(topic.nameIdentifier()); schemaMetaService.deleteSchema(schema.nameIdentifier(), false); } @@ -561,14 +646,24 @@ public void testInsertHierarchicalSecondLeafReusesAncestorsWithoutUpsert() throw .build(); schemaMetaService.insertSchema(first, false); - long idA = - schemaMetaService - .getSchemaByIdentifier(NameIdentifier.of(metalakeName, catalogName, ancestorA)) - .id(); - long idAB = - schemaMetaService - .getSchemaByIdentifier(NameIdentifier.of(metalakeName, catalogName, ancestorAB)) - .id(); + SchemaPO ancestorAPOBefore = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + mapper.selectSchemaMetaById( + schemaMetaService + .getSchemaByIdentifier( + NameIdentifier.of(metalakeName, catalogName, ancestorA)) + .id())); + SchemaPO ancestorABPOBefore = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + mapper.selectSchemaMetaById( + schemaMetaService + .getSchemaByIdentifier( + NameIdentifier.of(metalakeName, catalogName, ancestorAB)) + .id())); SchemaEntity second = SchemaEntity.builder() @@ -581,16 +676,41 @@ public void testInsertHierarchicalSecondLeafReusesAncestorsWithoutUpsert() throw .build(); schemaMetaService.insertSchema(second, false); + SchemaPO ancestorAPOAfter = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + mapper.selectSchemaMetaById( + schemaMetaService + .getSchemaByIdentifier( + NameIdentifier.of(metalakeName, catalogName, ancestorA)) + .id())); + SchemaPO ancestorABPOAfter = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + mapper.selectSchemaMetaById( + schemaMetaService + .getSchemaByIdentifier( + NameIdentifier.of(metalakeName, catalogName, ancestorAB)) + .id())); + Assertions.assertEquals(ancestorAPOBefore.getSchemaId(), ancestorAPOAfter.getSchemaId()); + Assertions.assertEquals(ancestorABPOBefore.getSchemaId(), ancestorABPOAfter.getSchemaId()); Assertions.assertEquals( - idA, - schemaMetaService - .getSchemaByIdentifier(NameIdentifier.of(metalakeName, catalogName, ancestorA)) - .id()); + ancestorAPOBefore.getCurrentVersion() + 1, ancestorAPOAfter.getCurrentVersion()); Assertions.assertEquals( - idAB, - schemaMetaService - .getSchemaByIdentifier(NameIdentifier.of(metalakeName, catalogName, ancestorAB)) - .id()); + ancestorABPOBefore.getCurrentVersion() + 1, ancestorABPOAfter.getCurrentVersion()); + } + + private SchemaEntity copySchemaWithComment(SchemaEntity schema, String comment) { + return SchemaEntity.builder() + .withId(schema.id()) + .withName(schema.name()) + .withNamespace(schema.namespace()) + .withComment(comment) + .withProperties(schema.properties()) + .withAuditInfo(schema.auditInfo()) + .build(); } private void associateTag(TagEntity tag, NameIdentifier ident, Entity.EntityType type) From 51b34700fc63224134f1e1c3849679c801bf500b Mon Sep 17 00:00:00 2001 From: yuqi Date: Wed, 5 Aug 2026 09:42:01 +0800 Subject: [PATCH 3/9] [#12342] test(fileset): initialize managed schema parents --- .../fileset/TestFilesetCatalogOperations.java | 127 ++++++++++-------- 1 file changed, 74 insertions(+), 53 deletions(-) diff --git a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java index 1a55d197921..89eb3c38911 100644 --- a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java +++ b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java @@ -59,6 +59,7 @@ import java.io.IOException; import java.net.ConnectException; import java.nio.file.Paths; +import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -102,6 +103,10 @@ import org.apache.gravitino.file.FileInfo; import org.apache.gravitino.file.Fileset; import org.apache.gravitino.file.FilesetChange; +import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.BaseMetalake; +import org.apache.gravitino.meta.CatalogEntity; +import org.apache.gravitino.meta.SchemaVersion; import org.apache.gravitino.storage.IdGenerator; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.RelationalEntityStore; @@ -221,7 +226,7 @@ private static CatalogInfo randomCatalogInfo( } @BeforeAll - public static void setUp() throws IllegalAccessException { + public static void setUp() throws IOException, IllegalAccessException { Config config = Mockito.mock(Config.class); when(config.get(ENTITY_STORE)).thenReturn(RELATIONAL_ENTITY_STORE); when(config.get(ENTITY_RELATIONAL_STORE)).thenReturn(DEFAULT_ENTITY_RELATIONAL_STORE); @@ -263,6 +268,28 @@ public static void setUp() throws IllegalAccessException { store.initialize(config); idGenerator = new RandomIdGenerator(); + AuditInfo auditInfo = + AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build(); + BaseMetalake metalake = + BaseMetalake.builder() + .withId(1L) + .withName("m1") + .withVersion(SchemaVersion.V_0_1) + .withAuditInfo(auditInfo) + .build(); + store.put(metalake, false); + + CatalogEntity catalog = + CatalogEntity.builder() + .withId(1L) + .withName("c1") + .withNamespace(Namespace.of("m1")) + .withProvider("fileset") + .withType(Catalog.Type.FILESET) + .withAuditInfo(auditInfo) + .build(); + store.put(catalog, false); + // Mock MetalakeMetaService metalakeMetaService = MetalakeMetaService.getInstance(); MetalakeMetaService spyMetaService = Mockito.spy(metalakeMetaService); @@ -402,14 +429,13 @@ public void testCreateSchemaWithNoLocation() throws IOException { final long testId = generateTestId(); final String name = "schema" + testId; final String comment = "comment" + testId; - Schema schema = createSchema(testId, name, comment, null, null); + Schema schema = createSchema(name, comment, null, null); Assertions.assertEquals(name, schema.name()); Assertions.assertEquals(comment, schema.comment()); Throwable exception = Assertions.assertThrows( - SchemaAlreadyExistsException.class, - () -> createSchema(testId, name, comment, null, null)); + SchemaAlreadyExistsException.class, () -> createSchema(name, comment, null, null)); Assertions.assertEquals( "Schema m1.c1.schema" + testId + " already exists", exception.getMessage()); } @@ -424,7 +450,7 @@ public void testCreateSchemaWithEmptyCatalogLocation() throws IOException { Throwable exception = Assertions.assertThrows( IllegalArgumentException.class, - () -> createSchema(testId, schemaName, comment, catalogPath, null)); + () -> createSchema(schemaName, comment, catalogPath, null)); Assertions.assertEquals( "The value of the catalog property " + FilesetCatalogPropertiesMetadata.LOCATION @@ -438,7 +464,7 @@ public void testCreateSchemaWithCatalogLocation() throws IOException { String name = "schema" + testId; final String comment = "comment" + testId; String catalogPath = TEST_ROOT_PATH + "/" + "catalog12"; - Schema schema = createSchema(testId, name, comment, catalogPath, null); + Schema schema = createSchema(name, comment, catalogPath, null); Assertions.assertEquals(name, schema.name()); Path schemaPath = new Path(catalogPath, name); @@ -450,7 +476,7 @@ public void testCreateSchemaWithCatalogLocation() throws IOException { // test placeholder in catalog location name = "schema" + testId + "_1"; catalogPath = TEST_ROOT_PATH + "/" + "{{catalog}}-{{schema}}"; - schema = createSchema(testId, name, comment, catalogPath, null); + schema = createSchema(name, comment, catalogPath, null); Assertions.assertEquals(name, schema.name()); schemaPath = new Path(catalogPath, name); @@ -460,7 +486,7 @@ public void testCreateSchemaWithCatalogLocation() throws IOException { // Test disable server-side FS operations. name = "schema" + testId + "_2"; catalogPath = TEST_ROOT_PATH + "/" + "catalog12_2"; - schema = createSchema(testId, name, comment, catalogPath, null, true); + schema = createSchema(name, comment, catalogPath, null, true); Assertions.assertEquals(name, schema.name()); // Schema path should not be existed if the server-side FS operations are disabled. @@ -475,7 +501,7 @@ public void testCreateSchemaWithSchemaLocation() throws IOException { final String comment = "comment" + testId; String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; String schemaPath = catalogPath + "/" + name; - Schema schema = createSchema(testId, name, comment, null, schemaPath); + Schema schema = createSchema(name, comment, null, schemaPath); Assertions.assertEquals(name, schema.name()); Path schemaPath1 = new Path(schemaPath); @@ -487,7 +513,7 @@ public void testCreateSchemaWithSchemaLocation() throws IOException { // test placeholder in schema location name = "schema" + testId + "_1"; schemaPath = catalogPath + "/" + "{{schema}}"; - schema = createSchema(testId, name, comment, null, schemaPath); + schema = createSchema(name, comment, null, schemaPath); Assertions.assertEquals(name, schema.name()); schemaPath1 = new Path(schemaPath); @@ -500,7 +526,7 @@ public void testCreateSchemaWithSchemaLocation() throws IOException { Throwable exception = Assertions.assertThrows( IllegalArgumentException.class, - () -> createSchema(testId, schemaName1, comment, null, schemaPath2)); + () -> createSchema(schemaName1, comment, null, schemaPath2)); Assertions.assertTrue( exception.getMessage().contains("Placeholder in location should not be empty"), exception.getMessage()); @@ -508,7 +534,7 @@ public void testCreateSchemaWithSchemaLocation() throws IOException { // Test disable server-side FS operations. name = "schema" + testId + "_3"; schemaPath = catalogPath + "/" + name; - schema = createSchema(testId, name, comment, null, schemaPath, true); + schema = createSchema(name, comment, null, schemaPath, true); Assertions.assertEquals(name, schema.name()); // Schema path should not be existed if the server-side FS operations are disabled. @@ -522,7 +548,7 @@ public void testCreateSchemaWithCatalogAndSchemaLocation() throws IOException { String comment = "comment" + testId; String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; String schemaPath = TEST_ROOT_PATH + "/" + "schema" + testId; - Schema schema = createSchema(testId, name, comment, catalogPath, schemaPath); + Schema schema = createSchema(name, comment, catalogPath, schemaPath); Assertions.assertEquals(name, schema.name()); Path schemaPath1 = new Path(schemaPath); @@ -538,7 +564,7 @@ public void testCreateSchemaWithCatalogAndSchemaLocation() throws IOException { name = "schema" + testId + "_1"; catalogPath = TEST_ROOT_PATH + "/" + "{{catalog}}"; schemaPath = TEST_ROOT_PATH + "/" + "{{schema}}"; - schema = createSchema(testId, name, comment, catalogPath, schemaPath); + schema = createSchema(name, comment, catalogPath, schemaPath); Assertions.assertEquals(name, schema.name()); schemaPath1 = new Path(schemaPath); @@ -551,7 +577,7 @@ public void testCreateSchemaWithCatalogAndSchemaLocation() throws IOException { name = "schema" + testId + "_2"; catalogPath = TEST_ROOT_PATH + "/" + "catalog14_2"; schemaPath = TEST_ROOT_PATH + "/" + "schema14_2"; - schema = createSchema(testId, name, comment, catalogPath, schemaPath, true); + schema = createSchema(name, comment, catalogPath, schemaPath, true); Assertions.assertEquals(name, schema.name()); // Schema path should not be existed if the server-side FS operations are disabled. @@ -565,7 +591,7 @@ public void testLoadSchema() throws IOException { String name = "schema" + testId; String comment = "comment" + testId; String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; - Schema schema = createSchema(testId, name, comment, catalogPath, null); + Schema schema = createSchema(name, comment, catalogPath, null); NameIdentifier otherSchema = NameIdentifierUtil.ofSchema("m1", "c1", "otherSchema"); Assertions.assertEquals(name, schema.name()); @@ -593,8 +619,8 @@ public void testListSchema() throws IOException { String comment1 = "comment" + testId1; String name2 = "schema" + testId2; String comment2 = "comment" + testId2; - createSchema(testId1, name1, comment1, null, null); - createSchema(testId2, name2, comment2, null, null); + createSchema(name1, comment1, null, null); + createSchema(name2, comment2, null, null); try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store)) { ops.initialize(Maps.newHashMap(), randomCatalogInfo(), FILESET_PROPERTIES_METADATA); @@ -612,7 +638,7 @@ public void testAlterSchema() throws IOException { String name = "schema" + testId; String comment = "comment" + testId; String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; - Schema schema = createSchema(testId, name, comment, catalogPath, null); + Schema schema = createSchema(name, comment, catalogPath, null); Assertions.assertEquals(name, schema.name()); try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store)) { @@ -660,7 +686,7 @@ public void testDropSchema() throws IOException { final String comment = "comment" + testId; final String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; - Schema schema = createSchema(testId, schemaName, comment, catalogPath, null); + Schema schema = createSchema(schemaName, comment, catalogPath, null); Assertions.assertEquals(schemaName, schema.name()); NameIdentifier id = NameIdentifierUtil.ofSchema("m1", "c1", schemaName); @@ -683,7 +709,7 @@ public void testDropSchema() throws IOException { Assertions.assertFalse(fs.exists(schemaPath)); // Test drop non-empty schema with cascade = false - createSchema(testId, schemaName, comment, catalogPath, null); + createSchema(schemaName, comment, catalogPath, null); Fileset fs1 = createFileset("fs1", schemaName, "comment", Fileset.Type.MANAGED, catalogPath, null); Path fs1Path = new Path(fs1.storageLocation()); @@ -699,7 +725,7 @@ public void testDropSchema() throws IOException { Assertions.assertFalse(fs.exists(fs1Path)); // Test drop both managed and external filesets - createSchema(testId, schemaName, comment, catalogPath, null); + createSchema(schemaName, comment, catalogPath, null); Fileset fs2 = createFileset("fs2", schemaName, "comment", Fileset.Type.MANAGED, catalogPath, null); Path fs2Path = new Path(fs2.storageLocation()); @@ -715,7 +741,7 @@ public void testDropSchema() throws IOException { Assertions.assertTrue(fs.exists(fs3Path)); // Test drop schema with different storage location - createSchema(testId, schemaName, comment, catalogPath, null); + createSchema(schemaName, comment, catalogPath, null); Path fs4Path = new Path(TEST_ROOT_PATH + "/fs4"); createFileset( "fs4", schemaName, "comment", Fileset.Type.MANAGED, catalogPath, fs4Path.toString()); @@ -732,7 +758,7 @@ public void testDropSchemaWithFSOpsDisabled() throws IOException { final String filesetName = "fileset" + testId; final String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; - Schema schema = createSchema(testId, schemaName, comment, catalogPath, null); + Schema schema = createSchema(schemaName, comment, catalogPath, null); Assertions.assertEquals(schemaName, schema.name()); NameIdentifier id = NameIdentifierUtil.ofSchema("m1", "c1", schemaName); @@ -748,7 +774,7 @@ public void testDropSchemaWithFSOpsDisabled() throws IOException { FileSystem fs = schemaPath.getFileSystem(new Configuration()); Assertions.assertTrue(fs.exists(schemaPath)); - createSchema(testId, schemaName, comment, catalogPath, null); + createSchema(schemaName, comment, catalogPath, null); Fileset fs1 = createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, catalogPath, null); Path fs1Path = new Path(fs1.storageLocation()); @@ -781,7 +807,7 @@ public void testCreateLoadAndDeleteFilesetWithLocations( try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store)) { ops.initialize(catalogProps, randomCatalogInfo("m1", "c1"), FILESET_PROPERTIES_METADATA); if (!ops.schemaExists(schemaIdent)) { - createSchema(generateTestId(), schemaName, comment, catalogPath, schemaPath); + createSchema(schemaName, comment, catalogPath, schemaPath); } Fileset fileset = createFileset(name, schemaName, "comment", type, catalogPath, storageLocation); @@ -838,7 +864,7 @@ public void testCreateLoadAndDeleteFilesetWithLocationsWhenFSOpsDisabled( try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store)) { ops.initialize(catalogProps, randomCatalogInfo("m1", "c1"), FILESET_PROPERTIES_METADATA); if (!ops.schemaExists(schemaIdent)) { - createSchema(generateTestId(), schemaName, comment, catalogPath, schemaPath, true); + createSchema(schemaName, comment, catalogPath, schemaPath, true); } Fileset fileset; @@ -896,7 +922,7 @@ public void testCreateFilesetWithExceptions() throws IOException { final String comment = "comment" + testId; final String filesetName = "fileset" + testId; - createSchema(testId, schemaName, comment, null, null); + createSchema(schemaName, comment, null, null); NameIdentifier filesetIdent = NameIdentifier.of("m1", "c1", schemaName, filesetName); // If neither catalog location, nor schema location and storageLocation is specified. @@ -946,7 +972,7 @@ public void testListFilesets() throws IOException { String comment = "comment" + testId; String schemaPath = TEST_ROOT_PATH + "/" + schemaName; - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); String[] filesets = { "fileset" + testId + "_1", "fileset" + testId + "_2", "fileset" + testId + "_3" @@ -976,7 +1002,7 @@ public void testListFilesetFiles() throws IOException { final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; final NameIdentifier filesetIdent = NameIdentifier.of("m1", "c1", schemaName, filesetName); - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, null, null); try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store)) { @@ -1026,7 +1052,7 @@ public void testListFilesetFilesWithFSOpsDisabled() throws Exception { final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; final NameIdentifier filesetIdent = NameIdentifier.of("m1", "c1", schemaName, filesetName); - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, null, null); Map catalogProps = Collections.singletonMap(DISABLE_FILESYSTEM_OPS, "true"); @@ -1053,7 +1079,7 @@ public void testListFilesetFilesWithNonExistentPath() throws IOException { String filesetName = "fileset" + testId; final String nonExistentSubPath = "/non_existent_file.txt"; - Schema schema = createSchema(testId, schemaName, comment, null, schemaPath); + Schema schema = createSchema(schemaName, comment, null, schemaPath); Fileset fileset = createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, null, null); final NameIdentifier filesetIdent = @@ -1095,7 +1121,7 @@ public void testRenameFileset( try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store)) { ops.initialize(catalogProps, randomCatalogInfo("m1", "c1"), FILESET_PROPERTIES_METADATA); if (!ops.schemaExists(schemaIdent)) { - createSchema(generateTestId(), schemaName, comment, catalogPath, schemaPath); + createSchema(schemaName, comment, catalogPath, schemaPath); } Fileset fileset = createFileset(name, schemaName, "comment", type, catalogPath, storageLocation); @@ -1133,7 +1159,7 @@ public void testAlterFilesetProperties() throws IOException { final String filesetName = "fileset" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); Fileset fileset = createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, null, null); @@ -1248,7 +1274,7 @@ public void testUpdateFilesetComment() throws IOException { final String name = "fileset" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); Fileset fileset = createFileset(name, schemaName, comment, Fileset.Type.MANAGED, null, null); FilesetChange change1 = FilesetChange.updateComment(comment + "_new"); @@ -1272,7 +1298,7 @@ public void testRemoveFilesetComment() throws IOException { final String filesetName = "fileset" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); Fileset fileset = createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, null, null); @@ -1346,7 +1372,7 @@ public void testGetFileLocation() throws IOException { final String storageLocation = TEST_ROOT_PATH + "/" + catalogName + "/" + schemaName + "/" + filesetName; - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); Fileset fileset = createFileset( filesetName, schemaName, comment, Fileset.Type.MANAGED, null, storageLocation); @@ -1510,7 +1536,7 @@ public void testCreateSchemaWithDifferentUser() throws Exception { final String schemaName = "schema" + testId; final String comment = "comment" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; - return createSchema(testId, schemaName, comment, null, schemaPath, false); + return createSchema(schemaName, comment, null, schemaPath, false); }); Assertions.assertNotNull(schemaCreatedByAlice); @@ -1526,7 +1552,7 @@ public void testCreateSchemaWithDifferentUser() throws Exception { final String comment = "comment" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; // Create schema with user "bob" - return createSchema(testId, schemaName, comment, null, schemaPath, false); + return createSchema(schemaName, comment, null, schemaPath, false); }); Assertions.assertNotNull(schemaCreatedByBob); Assertions.assertEquals("bob", schemaCreatedByBob.auditInfo().creator()); @@ -1541,7 +1567,7 @@ public void testCreateSchemaWithDifferentUser() throws Exception { final String comment = "comment" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; // Create schema with user "lucy" - return createSchema(testId, schemaName, comment, null, schemaPath, false); + return createSchema(schemaName, comment, null, schemaPath, false); }); Assertions.assertNotNull(schemaCreatedByLucy); Assertions.assertEquals("lucy", schemaCreatedByLucy.auditInfo().creator()); @@ -1555,7 +1581,7 @@ public void testLocationPlaceholdersWithException() throws IOException { final String filesetName = "fileset" + testId; String storageLocation = TEST_ROOT_PATH + "/{{fileset}}/{{user}}/{{id}}"; - createSchema(testId, schemaName, null, null, null); + createSchema(schemaName, null, null, null); Exception exception = Assertions.assertThrows( @@ -1612,7 +1638,7 @@ public void testPlaceholdersInLocation( try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store)) { ops.initialize(catalogProps, randomCatalogInfo("m1", "c1"), FILESET_PROPERTIES_METADATA); if (!ops.schemaExists(schemaIdent)) { - createSchema(generateTestId(), schemaName, comment, catalogPath, schemaPath); + createSchema(schemaName, comment, catalogPath, schemaPath); } Fileset fileset = createFileset( @@ -2927,22 +2953,17 @@ private static Stream testRenameArguments() { TEST_ROOT_PATH + "/fileset39")); } - private Schema createSchema( - long testId, String name, String comment, String catalogPath, String schemaPath) + private Schema createSchema(String name, String comment, String catalogPath, String schemaPath) throws IOException { - return createSchema(testId, name, comment, catalogPath, schemaPath, false); + return createSchema(name, comment, catalogPath, schemaPath, false); } private Schema createSchema( - long testId, - String name, - String comment, - String catalogPath, - String schemaPath, - boolean disableFsOps) + String name, String comment, String catalogPath, String schemaPath, boolean disableFsOps) throws IOException { + long schemaId = idGenerator.nextId(); // stub schema - doReturn(new SchemaIds(1L, 1L, testId)) + doReturn(new SchemaIds(1L, 1L, schemaId)) .when(spySchemaMetaService) .getSchemaIdByMetalakeNameAndCatalogNameAndSchemaName( Mockito.anyString(), Mockito.anyString(), Mockito.eq(name)); @@ -2958,7 +2979,7 @@ private Schema createSchema( NameIdentifier schemaIdent = NameIdentifierUtil.ofSchema("m1", "c1", name); Map schemaProps = Maps.newHashMap(); - StringIdentifier stringId = StringIdentifier.fromId(testId); + StringIdentifier stringId = StringIdentifier.fromId(schemaId); schemaProps = Maps.newHashMap(StringIdentifier.newPropertiesWithId(stringId, schemaProps)); if (schemaPath != null) { From 97e1fc897670a8194dab83adc423b911f08a736e Mon Sep 17 00:00:00 2001 From: yuqi Date: Wed, 5 Aug 2026 11:00:51 +0800 Subject: [PATCH 4/9] [#12342] test(kafka): persist managed catalog parents --- .../kafka/TestKafkaCatalogOperations.java | 60 ++++++++----------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/TestKafkaCatalogOperations.java b/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/TestKafkaCatalogOperations.java index dd30df96f3e..69fa82cacdf 100644 --- a/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/TestKafkaCatalogOperations.java +++ b/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/TestKafkaCatalogOperations.java @@ -46,7 +46,6 @@ import static org.apache.gravitino.catalog.kafka.KafkaCatalogPropertiesMetadata.BOOTSTRAP_SERVERS; import static org.apache.gravitino.catalog.kafka.KafkaTopicPropertiesMetadata.PARTITION_COUNT; import static org.apache.gravitino.catalog.kafka.KafkaTopicPropertiesMetadata.REPLICATION_FACTOR; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableMap; @@ -74,18 +73,16 @@ import org.apache.gravitino.messaging.Topic; import org.apache.gravitino.messaging.TopicChange; import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.BaseMetalake; import org.apache.gravitino.meta.CatalogEntity; +import org.apache.gravitino.meta.SchemaVersion; import org.apache.gravitino.storage.IdGenerator; import org.apache.gravitino.storage.RandomIdGenerator; -import org.apache.gravitino.storage.relational.helper.CatalogIds; -import org.apache.gravitino.storage.relational.service.CatalogMetaService; -import org.apache.gravitino.storage.relational.service.MetalakeMetaService; import org.apache.kafka.common.config.TopicConfig; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.mockito.MockedStatic; import org.mockito.Mockito; public class TestKafkaCatalogOperations extends KafkaClusterEmbedded { @@ -141,7 +138,7 @@ public PropertiesMetadata modelVersionPropertiesMetadata() private static KafkaCatalogOperations kafkaCatalogOperations; @BeforeAll - public static void setUp() throws IllegalAccessException { + public static void setUp() throws IOException, IllegalAccessException { Config config = Mockito.mock(Config.class); Mockito.when(config.get(STORE_TRANSACTION_MAX_SKEW_TIME)).thenReturn(1000L); Mockito.when(config.get(STORE_DELETE_AFTER_TIME)).thenReturn(20 * 60 * 1000L); @@ -178,35 +175,23 @@ public static void setUp() throws IllegalAccessException { Mockito.when(config.get(Configs.CACHE_IMPLEMENTATION)).thenReturn("caffeine"); Mockito.when(config.get(Configs.CACHE_LOCK_SEGMENTS)).thenReturn(16); - // Mock - MetalakeMetaService metalakeMetaService = MetalakeMetaService.getInstance(); - MetalakeMetaService spyMetaservice = Mockito.spy(metalakeMetaService); - doReturn(1L).when(spyMetaservice).getMetalakeIdByName(Mockito.anyString()); - - CatalogMetaService catalogMetaService = CatalogMetaService.getInstance(); - CatalogMetaService spyCatalogMetaService = Mockito.spy(catalogMetaService); - doReturn(1L) - .when(spyCatalogMetaService) - .getCatalogIdByMetalakeIdAndName(Mockito.anyLong(), Mockito.anyString()); - doReturn(new CatalogIds(1L, 1L)) - .when(spyCatalogMetaService) - .getCatalogIdByMetalakeAndCatalogName(Mockito.anyString(), Mockito.anyString()); - - MockedStatic metalakeMetaServiceMockedStatic = - Mockito.mockStatic(MetalakeMetaService.class); - MockedStatic catalogMetaServiceMockedStatic = - Mockito.mockStatic(CatalogMetaService.class); - - metalakeMetaServiceMockedStatic - .when(MetalakeMetaService::getInstance) - .thenReturn(spyMetaservice); - catalogMetaServiceMockedStatic - .when(CatalogMetaService::getInstance) - .thenReturn(spyCatalogMetaService); - store = EntityStoreFactory.createEntityStore(config); store.initialize(config); idGenerator = new RandomIdGenerator(); + + BaseMetalake metalake = + BaseMetalake.builder() + .withId(1L) + .withName(METALAKE_NAME) + .withVersion(SchemaVersion.V_0_1) + .withAuditInfo( + AuditInfo.builder() + .withCreator("testKafkaUser") + .withCreateTime(Instant.now()) + .build()) + .build(); + store.put(metalake, false); + kafkaCatalogEntity = CatalogEntity.builder() .withId(1L) @@ -221,6 +206,7 @@ public static void setUp() throws IllegalAccessException { .withCreateTime(Instant.now()) .build()) .build(); + store.put(kafkaCatalogEntity, false); FieldUtils.writeField(GravitinoEnv.getInstance(), "config", config, true); @@ -238,11 +224,11 @@ public static void tearDown() throws IOException { } @Test - public void testKafkaCatalogConfiguration() { + public void testKafkaCatalogConfiguration() throws IOException { String catalogName = "test_kafka_catalog_configuration"; CatalogEntity catalogEntity = CatalogEntity.builder() - .withId(2L) + .withId(idGenerator.nextId()) .withName(catalogName) .withNamespace(Namespace.of(METALAKE_NAME)) .withType(MESSAGING) @@ -254,6 +240,7 @@ public void testKafkaCatalogConfiguration() { .build()) .withProperties(MOCK_CATALOG_PROPERTIES) .build(); + store.put(catalogEntity, false); KafkaCatalogOperations ops = new KafkaCatalogOperations(store, idGenerator); Assertions.assertNull(ops.adminClientConfig); @@ -274,11 +261,11 @@ public void testKafkaCatalogConfiguration() { } @Test - public void testInitialization() { + public void testInitialization() throws IOException { String catalogName = "test_kafka_catalog_initialization"; CatalogEntity catalogEntity = CatalogEntity.builder() - .withId(2L) + .withId(idGenerator.nextId()) .withName(catalogName) .withNamespace(Namespace.of(METALAKE_NAME)) .withType(MESSAGING) @@ -290,6 +277,7 @@ public void testInitialization() { .build()) .withProperties(MOCK_CATALOG_PROPERTIES) .build(); + store.put(catalogEntity, false); KafkaCatalogOperations ops = new KafkaCatalogOperations(store, idGenerator); ops.initialize( MOCK_CATALOG_PROPERTIES, catalogEntity.toCatalogInfo(), KAFKA_PROPERTIES_METADATA); From 31a90c8840f191f081326a597df782487295e2e4 Mon Sep 17 00:00:00 2001 From: yuqi Date: Wed, 5 Aug 2026 17:58:53 +0800 Subject: [PATCH 5/9] [#12342] fix(core): close namespace OCC races --- .../relational/mapper/CatalogMetaMapper.java | 26 +++-- .../mapper/CatalogMetaSQLProviderFactory.java | 20 ++-- .../relational/mapper/MetalakeMetaMapper.java | 15 ++- .../MetalakeMetaSQLProviderFactory.java | 11 +- .../relational/mapper/SchemaMetaMapper.java | 14 +-- .../mapper/SchemaMetaSQLProviderFactory.java | 13 +-- .../base/CatalogMetaBaseSQLProvider.java | 28 ++--- .../base/MetalakeMetaBaseSQLProvider.java | 15 +-- .../base/SchemaMetaBaseSQLProvider.java | 28 +++-- .../CatalogMetaPostgreSQLProvider.java | 8 -- .../SchemaMetaPostgreSQLProvider.java | 16 --- .../service/CatalogMetaService.java | 16 +-- .../service/MetalakeMetaService.java | 29 ++++- .../relational/service/SchemaMetaService.java | 15 +-- .../service/TestCatalogMetaService.java | 97 ++++++++++++++++- .../service/TestMetalakeMetaService.java | 82 ++++++++++++++ .../service/TestSchemaMetaService.java | 101 +++++++++++++++++- 17 files changed, 383 insertions(+), 151 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java index d9250804ce1..1a2c59f5195 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java @@ -47,6 +47,12 @@ public interface CatalogMetaMapper { @SelectProvider(type = CatalogMetaSQLProviderFactory.class, method = "listCatalogPOsByMetalakeId") List listCatalogPOsByMetalakeId(@Param("metalakeId") Long metalakeId); + /** Selects and locks all active catalogs in a metalake for the current transaction. */ + @SelectProvider( + type = CatalogMetaSQLProviderFactory.class, + method = "listCatalogPOsByMetalakeIdForUpdate") + List listCatalogPOsByMetalakeIdForUpdate(@Param("metalakeId") Long metalakeId); + @SelectProvider(type = CatalogMetaSQLProviderFactory.class, method = "listCatalogPOsByCatalogIds") List listCatalogPOsByCatalogIds(@Param("catalogIds") List catalogIds); @@ -73,6 +79,12 @@ CatalogPO selectCatalogMetaByName( @SelectProvider(type = CatalogMetaSQLProviderFactory.class, method = "selectCatalogMetaById") CatalogPO selectCatalogMetaById(@Param("catalogId") Long catalogId); + /** Selects and locks an active catalog by ID for the current transaction. */ + @SelectProvider( + type = CatalogMetaSQLProviderFactory.class, + method = "selectCatalogMetaByIdForUpdate") + CatalogPO selectCatalogMetaByIdForUpdate(@Param("catalogId") Long catalogId); + @InsertProvider(type = CatalogMetaSQLProviderFactory.class, method = "insertCatalogMeta") void insertCatalogMeta(@Param("catalogMeta") CatalogPO catalogPO); @@ -86,15 +98,6 @@ Integer updateCatalogMeta( @Param("newCatalogMeta") CatalogPO newCatalogPO, @Param("oldCatalogMeta") CatalogPO oldCatalogPO); - /** - * Advances the catalog version when the expected OCC version still matches. - * - * @return the number of updated rows - */ - @UpdateProvider(type = CatalogMetaSQLProviderFactory.class, method = "fenceCatalogMeta") - Integer fenceCatalogMeta( - @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion); - @UpdateProvider( type = CatalogMetaSQLProviderFactory.class, method = "softDeleteCatalogMetasByCatalogId") @@ -111,11 +114,6 @@ Integer softDeleteCatalogMetasByCatalogId( method = "softDeleteCatalogMetasWithVersion") Integer softDeleteCatalogMetasWithVersion(@Param("catalogMetas") List catalogPOs); - @UpdateProvider( - type = CatalogMetaSQLProviderFactory.class, - method = "softDeleteCatalogMetasByMetalakeId") - Integer softDeleteCatalogMetasByMetalakeId(@Param("metalakeId") Long metalakeId); - @DeleteProvider( type = CatalogMetaSQLProviderFactory.class, method = "deleteCatalogMetasByLegacyTimeline") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java index 86467f6cef2..f55cbb43a0b 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java @@ -61,6 +61,11 @@ public static String listCatalogPOsByMetalakeId(@Param("metalakeId") Long metala return getProvider().listCatalogPOsByMetalakeId(metalakeId); } + /** Returns SQL that lists and locks all active catalogs in a metalake. */ + public static String listCatalogPOsByMetalakeIdForUpdate(@Param("metalakeId") Long metalakeId) { + return getProvider().listCatalogPOsByMetalakeIdForUpdate(metalakeId); + } + public static String listCatalogPOsByCatalogIds(@Param("catalogIds") List catalogIds) { return getProvider().listCatalogPOsByCatalogIds(catalogIds); } @@ -94,6 +99,11 @@ public static String selectCatalogMetaById(@Param("catalogId") Long catalogId) { return getProvider().selectCatalogMetaById(catalogId); } + /** Returns SQL that selects and locks an active catalog by ID. */ + public static String selectCatalogMetaByIdForUpdate(@Param("catalogId") Long catalogId) { + return getProvider().selectCatalogMetaByIdForUpdate(catalogId); + } + public static String insertCatalogMeta(@Param("catalogMeta") CatalogPO catalogPO) { return getProvider().insertCatalogMeta(catalogPO); } @@ -109,12 +119,6 @@ public static String updateCatalogMeta( return getProvider().updateCatalogMeta(newCatalogPO, oldCatalogPO); } - /** Returns SQL that advances a catalog OCC version conditionally. */ - public static String fenceCatalogMeta( - @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion) { - return getProvider().fenceCatalogMeta(catalogId, currentVersion); - } - public static String softDeleteCatalogMetasByCatalogId( @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion) { return getProvider().softDeleteCatalogMetasByCatalogId(catalogId, currentVersion); @@ -126,10 +130,6 @@ public static String softDeleteCatalogMetasWithVersion( return getProvider().softDeleteCatalogMetasWithVersion(catalogPOs); } - public static String softDeleteCatalogMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { - return getProvider().softDeleteCatalogMetasByMetalakeId(metalakeId); - } - public static String deleteCatalogMetasByLegacyTimeline( @Param("legacyTimeline") Long legacyTimeline, @Param("limit") int limit) { return getProvider().deleteCatalogMetasByLegacyTimeline(legacyTimeline, limit); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java index 4b40bc751ea..01a928fd70e 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java @@ -47,6 +47,12 @@ public interface MetalakeMetaMapper { @SelectProvider(type = MetalakeMetaSQLProviderFactory.class, method = "selectMetalakeMetaById") MetalakePO selectMetalakeMetaById(@Param("metalakeId") Long metalakeId); + /** Selects and locks an active metalake by ID for the current transaction. */ + @SelectProvider( + type = MetalakeMetaSQLProviderFactory.class, + method = "selectMetalakeMetaByIdForUpdate") + MetalakePO selectMetalakeMetaByIdForUpdate(@Param("metalakeId") Long metalakeId); + @SelectProvider( type = MetalakeMetaSQLProviderFactory.class, method = "listMetalakePOsByMetalakeIds") @@ -70,15 +76,6 @@ Integer updateMetalakeMeta( @Param("newMetalakeMeta") MetalakePO newMetalakePO, @Param("oldMetalakeMeta") MetalakePO oldMetalakePO); - /** - * Advances the metalake version when the expected OCC version still matches. - * - * @return the number of updated rows - */ - @UpdateProvider(type = MetalakeMetaSQLProviderFactory.class, method = "fenceMetalakeMeta") - Integer fenceMetalakeMeta( - @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion); - @UpdateProvider( type = MetalakeMetaSQLProviderFactory.class, method = "softDeleteMetalakeMetaByMetalakeId") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java index 79a91f41f7e..11f64ad662b 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java @@ -65,6 +65,11 @@ public static String selectMetalakeMetaById(@Param("metalakeId") Long metalakeId return getProvider().selectMetalakeMetaById(metalakeId); } + /** Returns SQL that selects and locks an active metalake by ID. */ + public static String selectMetalakeMetaByIdForUpdate(@Param("metalakeId") Long metalakeId) { + return getProvider().selectMetalakeMetaByIdForUpdate(metalakeId); + } + public static String selectMetalakeIdMetaByName(@Param("metalakeName") String metalakeName) { return getProvider().selectMetalakeIdMetaByName(metalakeName); } @@ -88,12 +93,6 @@ public static String updateMetalakeMeta( return getProvider().updateMetalakeMeta(newMetalakePO, oldMetalakePO); } - /** Returns SQL that advances a metalake OCC version conditionally. */ - public static String fenceMetalakeMeta( - @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { - return getProvider().fenceMetalakeMeta(metalakeId, currentVersion); - } - public static String softDeleteMetalakeMetaByMetalakeId( @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { return getProvider().softDeleteMetalakeMetaByMetalakeId(metalakeId, currentVersion); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java index 532a0b7d247..4d10694217e 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java @@ -42,6 +42,10 @@ public interface SchemaMetaMapper { @SelectProvider(type = SchemaMetaSQLProviderFactory.class, method = "listSchemaPOsByCatalogId") List listSchemaPOsByCatalogId(@Param("catalogId") Long catalogId); + /** Lists all active schemas in a metalake. */ + @SelectProvider(type = SchemaMetaSQLProviderFactory.class, method = "listSchemaPOsByMetalakeId") + List listSchemaPOsByMetalakeId(@Param("metalakeId") Long metalakeId); + @SelectProvider( type = SchemaMetaSQLProviderFactory.class, method = "listSchemaPOsByFullQualifiedName") @@ -132,16 +136,6 @@ Integer softDeleteSchemaMetaBySchemaIdAndVersion( method = "softDeleteSchemaMetasWithVersion") Integer softDeleteSchemaMetasWithVersion(@Param("schemaMetas") List schemaPOs); - @UpdateProvider( - type = SchemaMetaSQLProviderFactory.class, - method = "softDeleteSchemaMetasByMetalakeId") - Integer softDeleteSchemaMetasByMetalakeId(@Param("metalakeId") Long metalakeId); - - @UpdateProvider( - type = SchemaMetaSQLProviderFactory.class, - method = "softDeleteSchemaMetasByCatalogId") - Integer softDeleteSchemaMetasByCatalogId(@Param("catalogId") Long catalogId); - @DeleteProvider( type = SchemaMetaSQLProviderFactory.class, method = "deleteSchemaMetasByLegacyTimeline") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java index ba368c219fa..6c5bdf3c82b 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java @@ -72,6 +72,11 @@ public static String listSchemaPOsByCatalogId(@Param("catalogId") Long catalogId return getProvider().listSchemaPOsByCatalogId(catalogId); } + /** Returns SQL that lists all active schemas in a metalake. */ + public static String listSchemaPOsByMetalakeId(@Param("metalakeId") Long metalakeId) { + return getProvider().listSchemaPOsByMetalakeId(metalakeId); + } + public static String selectSchemaIdByCatalogIdAndName( @Param("catalogId") Long catalogId, @Param("schemaName") String name) { return getProvider().selectSchemaIdByCatalogIdAndName(catalogId, name); @@ -137,14 +142,6 @@ public static String softDeleteSchemaMetasWithVersion( return getProvider().softDeleteSchemaMetasWithVersion(schemaPOs); } - public static String softDeleteSchemaMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { - return getProvider().softDeleteSchemaMetasByMetalakeId(metalakeId); - } - - public static String softDeleteSchemaMetasByCatalogId(@Param("catalogId") Long catalogId) { - return getProvider().softDeleteSchemaMetasByCatalogId(catalogId); - } - public static String deleteSchemaMetasByLegacyTimeline( @Param("legacyTimeline") Long legacyTimeline, @Param("limit") int limit) { return getProvider().deleteSchemaMetasByLegacyTimeline(legacyTimeline, limit); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java index bdaee81ac3b..d086ad48da0 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java @@ -53,6 +53,11 @@ public String listCatalogPOsByMetalakeId(@Param("metalakeId") Long metalakeId) { + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; } + /** Returns SQL that lists and locks all active catalogs in a metalake. */ + public String listCatalogPOsByMetalakeIdForUpdate(@Param("metalakeId") Long metalakeId) { + return listCatalogPOsByMetalakeId(metalakeId) + " FOR UPDATE"; + } + public String listCatalogPOsByCatalogIds(@Param("catalogIds") List catalogIds) { return ""; } - public String softDeleteCatalogMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { - return "UPDATE " - + TABLE_NAME - + " SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)" - + " + EXTRACT(MICROSECOND FROM CURRENT_TIMESTAMP(3)) / 1000" - + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; - } - public String deleteCatalogMetasByLegacyTimeline( @Param("legacyTimeline") Long legacyTimeline, @Param("limit") int limit) { return "DELETE FROM " diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java index 5899b3d42b9..a7a78f4b7b5 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java @@ -59,6 +59,11 @@ public String selectMetalakeMetaById(@Param("metalakeId") Long metalakeId) { + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; } + /** Returns SQL that selects and locks an active metalake by ID. */ + public String selectMetalakeMetaByIdForUpdate(@Param("metalakeId") Long metalakeId) { + return selectMetalakeMetaById(metalakeId) + " FOR UPDATE"; + } + public String selectMetalakeIdMetaByName(@Param("metalakeName") String metalakeName) { return "SELECT metalake_id as metalakeId" + " FROM " @@ -147,16 +152,6 @@ public String updateMetalakeMeta( + " AND deleted_at = 0"; } - /** Returns SQL that advances a metalake OCC version conditionally. */ - public String fenceMetalakeMeta( - @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { - return "UPDATE " - + TABLE_NAME - + " SET last_version = current_version + 1, current_version = current_version + 1" - + " WHERE metalake_id = #{metalakeId}" - + " AND current_version = #{currentVersion} AND deleted_at = 0"; - } - public String softDeleteMetalakeMetaByMetalakeId( @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { return "UPDATE " diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java index df7fd809586..889738f9616 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java @@ -38,6 +38,18 @@ public String listSchemaPOsByCatalogId(@Param("catalogId") Long catalogId) { + " WHERE catalog_id = #{catalogId} AND deleted_at = 0"; } + /** Returns SQL that lists all active schemas in a metalake. */ + public String listSchemaPOsByMetalakeId(@Param("metalakeId") Long metalakeId) { + return "SELECT schema_id as schemaId, schema_name as schemaName," + + " metalake_id as metalakeId, catalog_id as catalogId," + + " schema_comment as schemaComment, properties, audit_info as auditInfo," + + " current_version as currentVersion, last_version as lastVersion," + + " deleted_at as deletedAt" + + " FROM " + + TABLE_NAME + + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; + } + public String listSchemaPOsByFullQualifiedName( @Param("metalakeName") String metalakeName, @Param("catalogName") String catalogName) { return """ @@ -325,22 +337,6 @@ public String softDeleteSchemaMetasWithVersion(@Param("schemaMetas") List"; } - public String softDeleteSchemaMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { - return "UPDATE " - + TABLE_NAME - + " SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)" - + " + EXTRACT(MICROSECOND FROM CURRENT_TIMESTAMP(3)) / 1000" - + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; - } - - public String softDeleteSchemaMetasByCatalogId(@Param("catalogId") Long catalogId) { - return "UPDATE " - + TABLE_NAME - + " SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)" - + " + EXTRACT(MICROSECOND FROM CURRENT_TIMESTAMP(3)) / 1000" - + " WHERE catalog_id = #{catalogId} AND deleted_at = 0"; - } - public String deleteSchemaMetasByLegacyTimeline( @Param("legacyTimeline") Long legacyTimeline, @Param("limit") int limit) { return "DELETE FROM " diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java index 462294d882c..248b67275e3 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java @@ -49,14 +49,6 @@ public String softDeleteCatalogMetasWithVersion(List catalogPOs) { + ""; } - @Override - public String softDeleteCatalogMetasByMetalakeId(Long metalakeId) { - return "UPDATE " - + TABLE_NAME - + " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000 AS BIGINT)" - + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; - } - @Override public String deleteCatalogMetasByLegacyTimeline( @Param("legacyTimeline") Long legacyTimeline, @Param("limit") int limit) { diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java index b451dd2b8de..13127fb0552 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java @@ -139,22 +139,6 @@ public String softDeleteSchemaMetasWithVersion(List schemaPOs) { + ""; } - @Override - public String softDeleteSchemaMetasByMetalakeId(Long metalakeId) { - return "UPDATE " - + TABLE_NAME - + " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000 AS BIGINT)" - + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; - } - - @Override - public String softDeleteSchemaMetasByCatalogId(Long catalogId) { - return "UPDATE " - + TABLE_NAME - + " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000 AS BIGINT)" - + " WHERE catalog_id = #{catalogId} AND deleted_at = 0"; - } - @Override public String deleteSchemaMetasByLegacyTimeline( @Param("legacyTimeline") Long legacyTimeline, @Param("limit") int limit) { diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java index a8886c56c27..68d8dfbc183 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java @@ -196,7 +196,7 @@ public void insertCatalog(CatalogEntity catalogEntity, boolean overwrite) throws } SessionUtils.doMultipleWithCommit( - () -> fenceMetalakeForCatalogCreate(metalakePO), + () -> lockMetalakeForCatalogCreate(metalakePO), () -> SessionUtils.doWithoutCommit( CatalogMetaMapper.class, @@ -437,17 +437,17 @@ private void deleteCatalogWithVersion( } } - private void fenceMetalakeForCatalogCreate(MetalakePO metalakePO) { - int fenced = + private void lockMetalakeForCatalogCreate(MetalakePO observedMetalakePO) { + MetalakePO currentMetalakePO = SessionUtils.getWithoutCommit( MetalakeMetaMapper.class, - mapper -> - mapper.fenceMetalakeMeta( - metalakePO.getMetalakeId(), metalakePO.getCurrentVersion())); - if (fenced == 0) { + mapper -> mapper.selectMetalakeMetaByIdForUpdate(observedMetalakePO.getMetalakeId())); + if (currentMetalakePO == null + || !Objects.equals( + currentMetalakePO.getMetalakeName(), observedMetalakePO.getMetalakeName())) { throw new OptimisticLockException( "The parent metalake %s was modified concurrently; retry the operation", - metalakePO.getMetalakeName()); + observedMetalakePO.getMetalakeName()); } } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java index 824279fa143..b639a0232d7 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java @@ -67,6 +67,7 @@ import org.apache.gravitino.storage.relational.mapper.ViewMetaMapper; import org.apache.gravitino.storage.relational.po.CatalogPO; import org.apache.gravitino.storage.relational.po.MetalakePO; +import org.apache.gravitino.storage.relational.po.SchemaPO; import org.apache.gravitino.storage.relational.po.cache.OperateType; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; @@ -235,11 +236,8 @@ public boolean deleteMetalake(NameIdentifier ident, boolean cascade) { () -> { deleteMetalakeWithVersion(ident, metalakeId, currentVersion); deleteCatalogsWithVersions(ident, metalakeId); + deleteSchemasWithVersions(ident, listSchemaPOsForCascade(metalakeId)); }, - () -> - SessionUtils.doWithoutCommit( - SchemaMetaMapper.class, - mapper -> mapper.softDeleteSchemaMetasByMetalakeId(metalakeId)), () -> SessionUtils.doWithoutCommit( TableMetaMapper.class, @@ -439,7 +437,8 @@ void deleteMetalakeWithVersion(NameIdentifier identifier, Long metalakeId, Long private void deleteCatalogsWithVersions(NameIdentifier metalakeIdentifier, Long metalakeId) { List catalogPOs = SessionUtils.getWithoutCommit( - CatalogMetaMapper.class, mapper -> mapper.listCatalogPOsByMetalakeId(metalakeId)); + CatalogMetaMapper.class, + mapper -> mapper.listCatalogPOsByMetalakeIdForUpdate(metalakeId)); if (catalogPOs.isEmpty()) { return; } @@ -454,6 +453,26 @@ private void deleteCatalogsWithVersions(NameIdentifier metalakeIdentifier, Long } } + List listSchemaPOsForCascade(Long metalakeId) { + return SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.listSchemaPOsByMetalakeId(metalakeId)); + } + + private void deleteSchemasWithVersions( + NameIdentifier metalakeIdentifier, List schemaPOs) { + if (schemaPOs.isEmpty()) { + return; + } + int deleted = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.softDeleteSchemaMetasWithVersion(schemaPOs)); + if (deleted != schemaPOs.size()) { + throw new OptimisticLockException( + "A schema under metalake %s was modified concurrently; retry the operation", + metalakeIdentifier); + } + } + private OptimisticLockException optimisticLockException(NameIdentifier identifier) { return new OptimisticLockException( "The metalake %s was modified concurrently; retry the operation", identifier); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java index f69ee3e96a5..6702b315aad 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java @@ -171,7 +171,7 @@ public void insertSchema(SchemaEntity schemaEntity, boolean overwrite) throws IO } SessionUtils.doMultipleWithCommit( - () -> fenceCatalogForSchemaCreate(catalogPO), + () -> lockCatalogForSchemaCreate(catalogPO), () -> SessionUtils.doWithoutCommit( SchemaMetaMapper.class, @@ -471,16 +471,17 @@ private List listSchemaPOs(Namespace namespace) { mapper -> POStorageReadRouting.listPOs(mapper, namespace, ops, Entity.EntityType.SCHEMA)); } - private void fenceCatalogForSchemaCreate(CatalogPO catalogPO) { - int fenced = + private void lockCatalogForSchemaCreate(CatalogPO observedCatalogPO) { + CatalogPO currentCatalogPO = SessionUtils.getWithoutCommit( CatalogMetaMapper.class, - mapper -> - mapper.fenceCatalogMeta(catalogPO.getCatalogId(), catalogPO.getCurrentVersion())); - if (fenced == 0) { + mapper -> mapper.selectCatalogMetaByIdForUpdate(observedCatalogPO.getCatalogId())); + if (currentCatalogPO == null + || !Objects.equals(currentCatalogPO.getCatalogName(), observedCatalogPO.getCatalogName()) + || !Objects.equals(currentCatalogPO.getMetalakeId(), observedCatalogPO.getMetalakeId())) { throw new OptimisticLockException( "The parent catalog %s was modified concurrently; retry the operation", - catalogPO.getCatalogName()); + observedCatalogPO.getCatalogName()); } } diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java index a6468fe5f32..267785549b8 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java @@ -29,7 +29,14 @@ import java.sql.SQLException; import java.sql.Statement; import java.time.Instant; +import java.util.Arrays; import java.util.List; +import java.util.Objects; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.apache.gravitino.Catalog; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; @@ -95,7 +102,7 @@ public void testInsertAlreadyExistsException() throws IOException { } @TestTemplate - public void testInsertCatalogFencesMetalakeAndRollsBackFenceOnFailure() throws IOException { + public void testInsertCatalogLocksMetalakeWithoutChangingVersion() throws IOException { MetalakePO beforeInsert = SessionUtils.getWithoutCommit( MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalakeName)); @@ -110,8 +117,8 @@ public void testInsertCatalogFencesMetalakeAndRollsBackFenceOnFailure() throws I MetalakePO afterInsert = SessionUtils.getWithoutCommit( MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalakeName)); - assertEquals(beforeInsert.getCurrentVersion() + 1, afterInsert.getCurrentVersion()); - assertEquals(afterInsert.getCurrentVersion(), afterInsert.getLastVersion()); + assertEquals(beforeInsert.getCurrentVersion(), afterInsert.getCurrentVersion()); + assertEquals(beforeInsert.getLastVersion(), afterInsert.getLastVersion()); CatalogEntity duplicate = createCatalog( @@ -128,6 +135,50 @@ public void testInsertCatalogFencesMetalakeAndRollsBackFenceOnFailure() throws I assertEquals(afterInsert.getLastVersion(), afterFailure.getLastVersion()); } + @TestTemplate + public void testConcurrentSameNameCatalogCreateReportsAlreadyExists() throws Exception { + CatalogEntity first = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "concurrent_catalog", + auditInfo); + CatalogEntity second = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + first.name(), + auditInfo); + + List results = insertCatalogsConcurrently(first, second); + assertEquals(1, results.stream().filter(Objects::isNull).count()); + Throwable failure = results.stream().filter(Objects::nonNull).findFirst().orElseThrow(); + Assertions.assertTrue( + failure instanceof EntityAlreadyExistsException, + () -> "Expected EntityAlreadyExistsException, but got " + failure); + } + + @TestTemplate + public void testConcurrentDifferentCatalogCreatesBothSucceed() throws Exception { + CatalogEntity first = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "concurrent_catalog_1", + auditInfo); + CatalogEntity second = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "concurrent_catalog_2", + auditInfo); + + List results = insertCatalogsConcurrently(first, second); + Assertions.assertTrue( + results.stream().allMatch(Objects::isNull), + () -> "Concurrent catalog creates failed: " + results); + } + @TestTemplate public void testUpdateAlreadyExistsException() throws IOException { CatalogEntity catalog = @@ -458,6 +509,46 @@ public void testDeleteCatalogCascadeRemovesTagRelations() throws IOException { assertEquals(0, countActiveTagRelForMetadataObject(function.id(), "FUNCTION")); } + private List insertCatalogsConcurrently(CatalogEntity first, CatalogEntity second) + throws Exception { + ExecutorService executor = Executors.newFixedThreadPool(2); + CountDownLatch ready = new CountDownLatch(2); + CountDownLatch start = new CountDownLatch(1); + try { + Future firstResult = + executor.submit( + () -> { + ready.countDown(); + start.await(); + try { + CatalogMetaService.getInstance().insertCatalog(first, false); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + Future secondResult = + executor.submit( + () -> { + ready.countDown(); + start.await(); + try { + CatalogMetaService.getInstance().insertCatalog(second, false); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + assertTrue(ready.await(30, TimeUnit.SECONDS)); + start.countDown(); + return Arrays.asList( + firstResult.get(30, TimeUnit.SECONDS), secondResult.get(30, TimeUnit.SECONDS)); + } finally { + start.countDown(); + executor.shutdownNow(); + } + } + private CatalogEntity copyCatalogWithComment(CatalogEntity catalog, String comment) { return CatalogEntity.builder() .withId(catalog.id()) diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java index d72650e8854..d5ed42880b0 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java @@ -25,20 +25,30 @@ import java.io.IOException; import java.time.Instant; import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.exceptions.NonEmptyEntityException; import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.BaseMetalake; +import org.apache.gravitino.meta.CatalogEntity; +import org.apache.gravitino.meta.SchemaEntity; import org.apache.gravitino.meta.SchemaVersion; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.TestJDBCBackend; import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper; +import org.apache.gravitino.storage.relational.mapper.SchemaMetaMapper; import org.apache.gravitino.storage.relational.po.MetalakePO; +import org.apache.gravitino.storage.relational.po.SchemaPO; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; +import org.apache.gravitino.utils.NamespaceUtil; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.TestTemplate; +import org.mockito.Mockito; public class TestMetalakeMetaService extends TestJDBCBackend { @@ -213,6 +223,78 @@ public void testDeleteReportsOptimisticLockConflict() throws IOException { assertTrue(backend.exists(metalake.nameIdentifier(), Entity.EntityType.METALAKE)); } + @TestTemplate + public void testCascadeDeleteReportsConcurrentSchemaAlter() throws Exception { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + CatalogEntity catalog = createAndInsertCatalog(METALAKE_NAME, "catalog"); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(METALAKE_NAME, catalog.name()), + "schema", + AUDIT_INFO); + backend.insert(schema, false); + SchemaPO schemaBeforeDelete = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); + + ExecutorService executor = Executors.newSingleThreadExecutor(); + MetalakeMetaService service = Mockito.spy(MetalakeMetaService.getInstance()); + try { + Mockito.doAnswer( + invocation -> { + Assertions.assertEquals(metalake.id(), invocation.getArgument(0)); + List schemaPOs = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> mapper.listSchemaPOsByMetalakeId(metalake.id())); + SchemaPO observedSchemaPO = + schemaPOs.stream() + .filter(schemaPO -> schemaPO.getSchemaId().equals(schema.id())) + .findFirst() + .orElseThrow(); + SchemaEntity competingSchema = + SchemaEntity.builder() + .withId(schema.id()) + .withName(schema.name()) + .withNamespace(schema.namespace()) + .withComment("competing update") + .withProperties(schema.properties()) + .withAuditInfo(schema.auditInfo()) + .build(); + SchemaPO competingSchemaPO = + POConverters.updateSchemaPOWithVersion(observedSchemaPO, competingSchema); + Future competingUpdate = + executor.submit( + () -> + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, + mapper -> + mapper.updateSchemaMeta(competingSchemaPO, observedSchemaPO))); + Assertions.assertEquals(1, competingUpdate.get(30, TimeUnit.SECONDS)); + return schemaPOs; + }) + .when(service) + .listSchemaPOsForCascade(metalake.id()); + + assertThrows( + OptimisticLockException.class, + () -> service.deleteMetalake(metalake.nameIdentifier(), true)); + } finally { + executor.shutdownNow(); + } + + assertTrue(backend.exists(metalake.nameIdentifier(), Entity.EntityType.METALAKE)); + assertTrue(backend.exists(catalog.nameIdentifier(), Entity.EntityType.CATALOG)); + assertTrue(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + SchemaPO schemaAfterDelete = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); + Assertions.assertEquals( + schemaBeforeDelete.getCurrentVersion() + 1, schemaAfterDelete.getCurrentVersion()); + Assertions.assertEquals("competing update", schemaAfterDelete.getSchemaComment()); + } + @TestTemplate public void testNonCascadeDeleteRollsBackMetalakeFence() throws IOException { BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java index 9e8a84ca566..27e4f5a318c 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java @@ -28,9 +28,16 @@ import java.sql.SQLException; import java.sql.Statement; import java.time.Instant; +import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; @@ -90,7 +97,7 @@ public void testInsertAlreadyExistsException() throws IOException { } @TestTemplate - public void testInsertSchemaFencesCatalogAndRollsBackFenceOnFailure() throws IOException { + public void testInsertSchemaLocksCatalogWithoutChangingVersion() throws IOException { createAndInsertMakeLake(metalakeName); CatalogEntity catalog = createAndInsertCatalog(metalakeName, catalogName); CatalogPO beforeInsert = @@ -107,8 +114,8 @@ public void testInsertSchemaFencesCatalogAndRollsBackFenceOnFailure() throws IOE CatalogPO afterInsert = SessionUtils.getWithoutCommit( CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); - Assertions.assertEquals(beforeInsert.getCurrentVersion() + 1, afterInsert.getCurrentVersion()); - Assertions.assertEquals(afterInsert.getCurrentVersion(), afterInsert.getLastVersion()); + Assertions.assertEquals(beforeInsert.getCurrentVersion(), afterInsert.getCurrentVersion()); + Assertions.assertEquals(beforeInsert.getLastVersion(), afterInsert.getLastVersion()); SchemaEntity duplicate = createSchemaEntity( @@ -125,6 +132,54 @@ public void testInsertSchemaFencesCatalogAndRollsBackFenceOnFailure() throws IOE Assertions.assertEquals(afterInsert.getLastVersion(), afterFailure.getLastVersion()); } + @TestTemplate + public void testConcurrentSameNameSchemaCreateReportsAlreadyExists() throws Exception { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity first = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "concurrent_schema", + AUDIT_INFO); + SchemaEntity second = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + first.name(), + AUDIT_INFO); + + List results = insertSchemasConcurrently(first, second); + Assertions.assertEquals(1, results.stream().filter(Objects::isNull).count()); + Throwable failure = results.stream().filter(Objects::nonNull).findFirst().orElseThrow(); + Assertions.assertTrue( + failure instanceof EntityAlreadyExistsException, + () -> "Expected EntityAlreadyExistsException, but got " + failure); + } + + @TestTemplate + public void testConcurrentDifferentSchemaCreatesBothSucceed() throws Exception { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity first = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "concurrent_schema_1", + AUDIT_INFO); + SchemaEntity second = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "concurrent_schema_2", + AUDIT_INFO); + + List results = insertSchemasConcurrently(first, second); + Assertions.assertTrue( + results.stream().allMatch(Objects::isNull), + () -> "Concurrent schema creates failed: " + results); + } + @TestTemplate public void testUpdateAlreadyExistsException() throws IOException { createAndInsertMakeLake(metalakeName); @@ -702,6 +757,46 @@ public void testInsertHierarchicalSecondLeafReusesAncestorsWithoutUpsert() throw ancestorABPOBefore.getCurrentVersion() + 1, ancestorABPOAfter.getCurrentVersion()); } + private List insertSchemasConcurrently(SchemaEntity first, SchemaEntity second) + throws Exception { + ExecutorService executor = Executors.newFixedThreadPool(2); + CountDownLatch ready = new CountDownLatch(2); + CountDownLatch start = new CountDownLatch(1); + try { + Future firstResult = + executor.submit( + () -> { + ready.countDown(); + start.await(); + try { + SchemaMetaService.getInstance().insertSchema(first, false); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + Future secondResult = + executor.submit( + () -> { + ready.countDown(); + start.await(); + try { + SchemaMetaService.getInstance().insertSchema(second, false); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + assertTrue(ready.await(30, TimeUnit.SECONDS)); + start.countDown(); + return Arrays.asList( + firstResult.get(30, TimeUnit.SECONDS), secondResult.get(30, TimeUnit.SECONDS)); + } finally { + start.countDown(); + executor.shutdownNow(); + } + } + private SchemaEntity copySchemaWithComment(SchemaEntity schema, String comment) { return SchemaEntity.builder() .withId(schema.id()) From 11d7aa8059c8dc8f2485e79b91e643b87248502f Mon Sep 17 00:00:00 2001 From: yuqi Date: Sun, 9 Aug 2026 14:02:57 +0800 Subject: [PATCH 6/9] [#12342] fix(core): harden namespace OCC locking --- .../relational/mapper/CatalogMetaMapper.java | 6 + .../mapper/CatalogMetaSQLProviderFactory.java | 13 +- .../relational/mapper/MetalakeMetaMapper.java | 6 + .../MetalakeMetaSQLProviderFactory.java | 13 +- .../relational/mapper/SchemaMetaMapper.java | 15 +- .../mapper/SchemaMetaSQLProviderFactory.java | 11 +- .../base/CatalogMetaBaseSQLProvider.java | 5 + .../base/MetalakeMetaBaseSQLProvider.java | 5 + .../base/SchemaMetaBaseSQLProvider.java | 15 +- .../CatalogMetaPostgreSQLProvider.java | 5 + .../MetalakeMetaPostgreSQLProvider.java | 5 + .../service/CatalogMetaService.java | 55 +++--- .../service/MetalakeMetaService.java | 30 ++- .../relational/service/SchemaMetaService.java | 106 ++++++++--- .../service/TestCatalogMetaService.java | 32 ++++ .../service/TestMetalakeMetaService.java | 90 +++++++++ .../service/TestSchemaMetaService.java | 178 +++++++++++++++++- 17 files changed, 504 insertions(+), 86 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java index 1a2c59f5195..854c7dd20ed 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaMapper.java @@ -85,6 +85,12 @@ CatalogPO selectCatalogMetaByName( method = "selectCatalogMetaByIdForUpdate") CatalogPO selectCatalogMetaByIdForUpdate(@Param("catalogId") Long catalogId); + /** Selects and share-locks an active catalog by ID for the current transaction. */ + @SelectProvider( + type = CatalogMetaSQLProviderFactory.class, + method = "selectCatalogMetaByIdForShare") + CatalogPO selectCatalogMetaByIdForShare(@Param("catalogId") Long catalogId); + @InsertProvider(type = CatalogMetaSQLProviderFactory.class, method = "insertCatalogMeta") void insertCatalogMeta(@Param("catalogMeta") CatalogPO catalogPO); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java index f55cbb43a0b..2bc9c594318 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java @@ -51,7 +51,13 @@ public static CatalogMetaBaseSQLProvider getProvider() { static class CatalogMetaMySQLProvider extends CatalogMetaBaseSQLProvider {} - static class CatalogMetaH2Provider extends CatalogMetaBaseSQLProvider {} + static class CatalogMetaH2Provider extends CatalogMetaBaseSQLProvider { + @Override + public String selectCatalogMetaByIdForShare(Long catalogId) { + // H2 has no shared row-lock syntax, so use an exclusive lock in tests. + return selectCatalogMetaByIdForUpdate(catalogId); + } + } public static String listCatalogPOsByMetalakeName(@Param("metalakeName") String metalakeName) { return getProvider().listCatalogPOsByMetalakeName(metalakeName); @@ -104,6 +110,11 @@ public static String selectCatalogMetaByIdForUpdate(@Param("catalogId") Long cat return getProvider().selectCatalogMetaByIdForUpdate(catalogId); } + /** Returns SQL that selects and share-locks an active catalog by ID. */ + public static String selectCatalogMetaByIdForShare(@Param("catalogId") Long catalogId) { + return getProvider().selectCatalogMetaByIdForShare(catalogId); + } + public static String insertCatalogMeta(@Param("catalogMeta") CatalogPO catalogPO) { return getProvider().insertCatalogMeta(catalogPO); } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java index 01a928fd70e..6479229aa3d 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java @@ -53,6 +53,12 @@ public interface MetalakeMetaMapper { method = "selectMetalakeMetaByIdForUpdate") MetalakePO selectMetalakeMetaByIdForUpdate(@Param("metalakeId") Long metalakeId); + /** Selects and share-locks an active metalake by ID for the current transaction. */ + @SelectProvider( + type = MetalakeMetaSQLProviderFactory.class, + method = "selectMetalakeMetaByIdForShare") + MetalakePO selectMetalakeMetaByIdForShare(@Param("metalakeId") Long metalakeId); + @SelectProvider( type = MetalakeMetaSQLProviderFactory.class, method = "listMetalakePOsByMetalakeIds") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java index 11f64ad662b..2a69fcd6273 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java @@ -51,7 +51,13 @@ public static MetalakeMetaBaseSQLProvider getProvider() { static class MetalakeMetaMySQLProvider extends MetalakeMetaBaseSQLProvider {} - static class MetalakeMetaH2Provider extends MetalakeMetaBaseSQLProvider {} + static class MetalakeMetaH2Provider extends MetalakeMetaBaseSQLProvider { + @Override + public String selectMetalakeMetaByIdForShare(Long metalakeId) { + // H2 has no shared row-lock syntax, so use an exclusive lock in tests. + return selectMetalakeMetaByIdForUpdate(metalakeId); + } + } public String listMetalakePOs() { return getProvider().listMetalakePOs(); @@ -70,6 +76,11 @@ public static String selectMetalakeMetaByIdForUpdate(@Param("metalakeId") Long m return getProvider().selectMetalakeMetaByIdForUpdate(metalakeId); } + /** Returns SQL that selects and share-locks an active metalake by ID. */ + public static String selectMetalakeMetaByIdForShare(@Param("metalakeId") Long metalakeId) { + return getProvider().selectMetalakeMetaByIdForShare(metalakeId); + } + public static String selectMetalakeIdMetaByName(@Param("metalakeName") String metalakeName) { return getProvider().selectMetalakeIdMetaByName(metalakeName); } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java index 4d10694217e..fa1e2fe6cc4 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java @@ -86,6 +86,12 @@ SchemaPO selectSchemaByFullQualifiedName( @SelectProvider(type = SchemaMetaSQLProviderFactory.class, method = "selectSchemaMetaById") SchemaPO selectSchemaMetaById(@Param("schemaId") Long schemaId); + /** Selects and locks an active schema by ID for the current transaction. */ + @SelectProvider( + type = SchemaMetaSQLProviderFactory.class, + method = "selectSchemaMetaByIdForUpdate") + SchemaPO selectSchemaMetaByIdForUpdate(@Param("schemaId") Long schemaId); + @InsertProvider(type = SchemaMetaSQLProviderFactory.class, method = "insertSchemaMeta") void insertSchemaMeta(@Param("schemaMeta") SchemaPO schemaPO); @@ -106,15 +112,6 @@ SchemaPO selectSchemaByFullQualifiedName( Integer updateSchemaMeta( @Param("newSchemaMeta") SchemaPO newSchemaPO, @Param("oldSchemaMeta") SchemaPO oldSchemaPO); - /** - * Advances the schema version when the expected OCC version still matches. - * - * @return the number of updated rows - */ - @UpdateProvider(type = SchemaMetaSQLProviderFactory.class, method = "fenceSchemaMeta") - Integer fenceSchemaMeta( - @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion); - @UpdateProvider( type = SchemaMetaSQLProviderFactory.class, method = "softDeleteSchemaMetasBySchemaIds") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java index 6c5bdf3c82b..0225084be33 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java @@ -98,6 +98,11 @@ public static String selectSchemaMetaById(@Param("schemaId") Long schemaId) { return getProvider().selectSchemaMetaById(schemaId); } + /** Returns SQL that selects and locks an active schema by ID. */ + public static String selectSchemaMetaByIdForUpdate(@Param("schemaId") Long schemaId) { + return getProvider().selectSchemaMetaByIdForUpdate(schemaId); + } + public static String insertSchemaMeta(@Param("schemaMeta") SchemaPO schemaPO) { return getProvider().insertSchemaMeta(schemaPO); } @@ -121,12 +126,6 @@ public static String updateSchemaMeta( return getProvider().updateSchemaMeta(newSchemaPO, oldSchemaPO); } - /** Returns SQL that advances a schema OCC version conditionally. */ - public static String fenceSchemaMeta( - @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion) { - return getProvider().fenceSchemaMeta(schemaId, currentVersion); - } - public static String softDeleteSchemaMetasBySchemaIds(@Param("schemaIds") List schemaIds) { return getProvider().softDeleteSchemaMetasBySchemaIds(schemaIds); } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java index d086ad48da0..43c213fd454 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java @@ -147,6 +147,11 @@ public String selectCatalogMetaByIdForUpdate(@Param("catalogId") Long catalogId) return selectCatalogMetaById(catalogId) + " FOR UPDATE"; } + /** Returns SQL that selects and share-locks an active catalog by ID. */ + public String selectCatalogMetaByIdForShare(@Param("catalogId") Long catalogId) { + return selectCatalogMetaById(catalogId) + " LOCK IN SHARE MODE"; + } + public String insertCatalogMeta(@Param("catalogMeta") CatalogPO catalogPO) { return "INSERT INTO " + TABLE_NAME diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java index a7a78f4b7b5..f301c90f1a7 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java @@ -64,6 +64,11 @@ public String selectMetalakeMetaByIdForUpdate(@Param("metalakeId") Long metalake return selectMetalakeMetaById(metalakeId) + " FOR UPDATE"; } + /** Returns SQL that selects and share-locks an active metalake by ID. */ + public String selectMetalakeMetaByIdForShare(@Param("metalakeId") Long metalakeId) { + return selectMetalakeMetaById(metalakeId) + " LOCK IN SHARE MODE"; + } + public String selectMetalakeIdMetaByName(@Param("metalakeName") String metalakeName) { return "SELECT metalake_id as metalakeId" + " FROM " diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java index 889738f9616..43ee9f472f7 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java @@ -182,6 +182,11 @@ public String selectSchemaMetaById(@Param("schemaId") Long schemaId) { + " WHERE schema_id = #{schemaId} AND deleted_at = 0"; } + /** Returns SQL that selects and locks an active schema by ID. */ + public String selectSchemaMetaByIdForUpdate(@Param("schemaId") Long schemaId) { + return selectSchemaMetaById(schemaId) + " FOR UPDATE"; + } + public String insertSchemaMeta(@Param("schemaMeta") SchemaPO schemaPO) { return "INSERT INTO " + TABLE_NAME @@ -289,16 +294,6 @@ public String updateSchemaMeta( + " AND deleted_at = 0"; } - /** Returns SQL that advances a schema OCC version conditionally. */ - public String fenceSchemaMeta( - @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion) { - return "UPDATE " - + TABLE_NAME - + " SET last_version = current_version + 1, current_version = current_version + 1" - + " WHERE schema_id = #{schemaId}" - + " AND current_version = #{currentVersion} AND deleted_at = 0"; - } - public String softDeleteSchemaMetasBySchemaIds(@Param("schemaIds") List schemaIds) { return "