yuqi1129 commented on code in PR #12350:
URL: https://github.com/apache/gravitino/pull/12350#discussion_r3719660941
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java:
##########
@@ -206,30 +210,32 @@ public <E extends Entity & HasIdentifier> BaseMetalake
updateMetalake(
throw re;
}
- if (updateResult.get() > 0) {
- return newMetalakeEntity;
- } else {
- throw new IOException("Failed to update the entity: " + ident);
- }
+ return newMetalakeEntity;
}
@Monitored(
metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
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)),
- () ->
- SessionUtils.doWithoutCommit(
- CatalogMetaMapper.class,
- mapper ->
mapper.softDeleteCatalogMetasByMetalakeId(metalakeId)),
+ () -> {
+ deleteMetalakeWithVersion(ident, metalakeId, currentVersion);
+ deleteCatalogsWithVersions(ident, metalakeId);
Review Comment:
Good catch. The metalake cascade now locks the active catalog parent rows
first, collects the active SchemaPO snapshot, CAS-deletes schemas by schema_id
and current_version, and validates the affected-row count. A concurrent schema
alter after the snapshot now raises OptimisticLockException and rolls back the
metalake/catalog deletion. Locking the catalog rows also prevents a concurrent
schema create from being missed under MySQL REPEATABLE READ. I added a
deterministic interleaving test that commits a schema alter between the
snapshot and batch delete and verifies the full cascade rolls back. Fixed in
31a90c8840.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java:
##########
@@ -492,13 +471,78 @@ private List<SchemaPO> 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()));
Review Comment:
Good catch. I replaced the parent version fence with SELECT ... FOR UPDATE
plus identity revalidation in the same transaction. The lock serializes schema
creation with catalog rename/delete, but it does not advance the catalog
version. As a result, concurrent same-name creates both reach the insert
sequentially and the loser is reported as EntityAlreadyExistsException, while
unrelated sibling creates both succeed. I added true concurrent tests for both
cases and applied the same strategy to catalog creation under a metalake. Fixed
in 31a90c8840.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
