yuqi1129 commented on code in PR #13436:
URL: https://github.com/apache/gravitino/pull/13436#discussion_r4093276398
##########
core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java:
##########
@@ -401,22 +401,32 @@ public boolean dropMetalake(NameIdentifier ident, boolean
force)
*
* <p>Callers typically {@code disableMetalake} before force-drop. {@link
* CatalogManager#dropCatalog} requires catalog {@code
metalake-in-use=true}, so a disabled
- * metalake is briefly re-enabled for child cleanup. The metalake entity is
deleted immediately
- * afterward, so the temporary enable is not restored.
+ * metalake is briefly re-enabled for child cleanup. On success the metalake
entity is deleted
+ * immediately afterward, so the temporary enable is not restored; if the
cleanup fails, the
+ * metalake is re-disabled (best effort) so a user-disabled metalake does
not stay enabled.
*/
private void dropCatalogsUnderMetalake(NameIdentifier metalakeIdent) {
if (catalogManager == null) {
return;
}
try {
- if (!metalakeInUse(store, metalakeIdent)) {
- enableMetalake(metalakeIdent);
- }
- List<CatalogEntity> catalogs =
- store.list(Namespace.of(metalakeIdent.name()), CatalogEntity.class,
EntityType.CATALOG);
- for (CatalogEntity catalog : catalogs) {
- catalogManager.dropCatalog(
- NameIdentifier.of(metalakeIdent.name(), catalog.name()), true /*
force */);
+ boolean wasDisabled = !metalakeInUse(store, metalakeIdent);
+ try {
+ if (wasDisabled) {
Review Comment:
There is still a problem. Considering the following case:
1. The user executes disableMetalake, at this time in-use=false.
2. The user executes dropMetalake(ident, force=true).
3. Enter phase 1: wasDisabled=true, call enableMetalake, at this time
in-use=true.
4. All catalogs are dropped successfully, there are no exceptions in the
try, and restore will not be executed. The method returns normally, metalake is
still in-use=true, and the wasDisabled information has been lost when the
method ends.
5. Entering stage 2: store.delete(...) fails. For example, an IOException is
thrown when the database connection is disconnected, and is wrapped into a
RuntimeException and thrown to the caller.
6. There is no recovery logic in phase 2, and it has no idea that phase 1
was ever temporarily enabled.
--
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]
