yuqi1129 commented on code in PR #12350:
URL: https://github.com/apache/gravitino/pull/12350#discussion_r3766735697
##########
core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java:
##########
@@ -358,7 +358,9 @@ public boolean dropMetalake(NameIdentifier ident, boolean
force)
}
return store.delete(ident, EntityType.METALAKE, true);
- } catch (NoSuchMetalakeException e) {
+ } catch (NoSuchMetalakeException | NoSuchEntityException e) {
Review Comment:
First, `catalogCache` is not an entity cache. It caches `CatalogWrapper`
objects, which hold the live catalog instance and its class loader. A metalake
has nothing like this, and `MetalakeManager` has no cache field
at all. The "preload all metalakes" comment in the constructor talks about
the entity cache in the store, not a cache in the manager.
Second, the entity cache in the store is already invalidated for us.
`RelationalEntityStore.delete()` calls `cache.invalidate(ident, entityType)` in
a `finally` block, so it runs both when the delete succeeds and when
the entity is already gone. So there is no stale entry left for this branch
to clean up.
Also, in this method the `NoSuchMetalakeException` / `NoSuchEntityException`
mostly comes from `metalakeInUse()` and `store.list()`, which run before we
reach the delete.
--
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]
