yuqi1129 commented on code in PR #12454:
URL: https://github.com/apache/gravitino/pull/12454#discussion_r3793467148
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java:
##########
@@ -318,18 +320,18 @@ public boolean deleteMetalake(NameIdentifier ident,
boolean cascade) {
ViewMetaMapper.class,
mapper ->
mapper.softDeleteViewMetasByMetalakeId(metalakeId)));
} else {
- List<CatalogEntity> 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(
- () ->
- SessionUtils.doWithoutCommit(
- MetalakeMetaMapper.class,
- mapper ->
mapper.softDeleteMetalakeMetaByMetalakeId(metalakeId)),
+ () -> {
+ deleteMetalakeWithVersion(ident, metalakeId, currentVersion);
+ List<CatalogPO> 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);
+ }
Review Comment:
The order is intentional because the soft delete also locks the metalake row.
The delete and the catalog check run in the same transaction. The delete is
not committed before the check. If catalogs exist, NonEmptyEntityException
causes the whole transaction to roll back, so the metalake is not deleted.
This works together with #12455, where catalog creation takes a shared lock
on the parent metalake row.
If we check catalogs first, this can happen:
1. Metalake drop checks catalogs and finds none.
2. Catalog create takes a shared lock on the metalake, inserts a catalog,
and commits.
3. Metalake drop deletes the metalake and commits.
We then have an active catalog under a deleted metalake. There is no foreign
key to prevent this. With the current order:
1. Metalake drop soft-deletes the metalake and locks its row, but does not
commit yet.
2. A catalog create waits when it tries to take the shared lock.
3. Metalake drop checks the catalogs.
If there are no catalogs, the drop commits. The catalog create then wakes
up, sees that the metalake has been deleted, and fails. If catalogs exist, the
drop throws NonEmptyEntityException and rolls back the soft delete.
If catalog creation gets the shared lock first, the metalake drop waits.
After the catalog creation commits, the drop continues, finds the catalog, and
rolls back. Therefore, the CAS UPDATE is used both to check the metalake
version and to lock the metalake row. It also avoids an extra SELECT FOR UPDATE.
I agree that this is not clear from this PR alone because the catalog-create
side of this lock rule is added in #12455. I will add a comment explaining this
relationship and add a concurrent catalog-create/metalake-drop test.
--
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]
