jerryshao commented on code in PR #12350:
URL: https://github.com/apache/gravitino/pull/12350#discussion_r3765897211
##########
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:
`CatalogManager.dropCatalog`'s equivalent new catch block also invalidates
`catalogCache` before returning `false` (to discard the now-stale cache entry).
This branch doesn't do the same for whatever metalake-level cache exists — is
that intentional (metalakes aren't cached the same way) or a gap?
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java:
##########
@@ -492,13 +470,151 @@ private List<SchemaPO> listSchemaPOs(Namespace
namespace) {
mapper -> POStorageReadRouting.listPOs(mapper, namespace, ops,
Entity.EntityType.SCHEMA));
}
+ private void lockCatalogForSchemaCreate(
+ CatalogPO observedCatalogPO, boolean createsImplicitAncestors) {
+ CatalogPO currentCatalogPO =
+ SessionUtils.getWithoutCommit(
+ CatalogMetaMapper.class,
+ mapper ->
+ createsImplicitAncestors
+ ?
mapper.selectCatalogMetaByIdForUpdate(observedCatalogPO.getCatalogId())
Review Comment:
Worth flagging as a design tradeoff: any hierarchical schema create (one
that materializes implicit ancestors) takes an *exclusive* lock on the whole
catalog row, which will block **all** other schema creates under that catalog —
not just ones touching the same ancestor path — for the duration of the
transaction. That's the right call for correctness, but it means catalogs with
heavy concurrent hierarchical schema creation will serialize on this lock.
Worth calling out explicitly in the PR description as an accepted throughput
tradeoff.
--
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]
