yuqi1129 commented on code in PR #12350:
URL: https://github.com/apache/gravitino/pull/12350#discussion_r3766731290
##########
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:
You are right. When a create needs to make implicit ancestors, we take an
exclusive lock on the catalog row, so all other schema creates under the same
catalog have to wait, even if they use a different path.
We need the exclusive lock here because two concurrent creates can both see
that an ancestor is missing and both insert it. Under MySQL REPEATABLE READ, a
shared lock is not enough to stop this.
So this is an accepted tradeoff: correctness first, less concurrency for
hierarchical creates. I will add this to the PR description.
If it becomes a problem later, we can make the lock smaller: only lock the
ancestor rows we really need to create, and use the unique constraint plus a
retry instead of locking the whole catalog row. I can do that in a follow-up
issue.
--
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]
