jerryshao commented on code in PR #12455:
URL: https://github.com/apache/gravitino/pull/12455#discussion_r3802952406
##########
core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java:
##########
@@ -984,6 +984,13 @@ public boolean dropCatalog(NameIdentifier ident, boolean
force)
} catch (NoSuchMetalakeException | NoSuchCatalogException ignored) {
return false;
+ } catch (NoSuchEntityException ignored) {
Review Comment:
`dropCatalog` now correctly converts the new `NoSuchEntityException` (thrown
when a concurrent metalake drop races the store read) into a plain `false`
return. `createCatalog()` (~line 642, not touched by this PR) doesn't get the
same treatment: `CatalogMetaService.insertCatalog` can now throw this same
`NoSuchEntityException` via `lockMetalakeForCatalogCreate` when the metalake is
dropped/renamed mid-create, but `createCatalog`'s catch chain only
special-cases `IllegalArgumentException`/`NoSuchMetalakeException` before
falling into a generic `catch (Exception e3)` that rethrows unchanged. Since
it's not a `NotFoundException` subtype, it ends up mapped to HTTP 500 instead
of the expected 404. Worth adding an equivalent `catch (NoSuchEntityException
...)` → `NoSuchCatalogException`/`NoSuchMetalakeException` conversion there.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java:
##########
@@ -374,6 +401,98 @@ public boolean deleteCatalog(NameIdentifier identifier,
boolean cascade) {
return true;
}
+ /**
Review Comment:
Nit: the new private helpers (`deleteCatalogWithVersion`,
`lockMetalakeForCatalogCreate`, `catalogWriteFailure`,
`deleteSchemasWithVersions`) are inserted between public methods
(`deleteCatalog` above and
`deleteCatalogMetasByLegacyTimeline`/`batchGetCatalogByIdentifier` below).
Project convention groups private methods at the end, after all public methods.
--
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]
