Copilot commented on code in PR #12603:
URL: https://github.com/apache/gravitino/pull/12603#discussion_r4095134593
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/SemanticModelMetaService.java:
##########
@@ -156,11 +186,246 @@ public void insertSemanticModel(SemanticModelEntity
semanticModelEntity, boolean
}
}
+ /**
+ * Atomically creates a complete new snapshot and advances the current
version pointer.
+ *
+ * @param identifier The current Semantic Model identifier.
+ * @param updater The entity updater.
+ * @param <E> The internal entity type accepted by the updater.
+ * @return The updated Semantic Model entity.
+ * @throws IOException If persistence fails.
+ * @throws OptimisticLockException If the internal transaction loses a
concurrent update race.
+ */
+ @Monitored(
+ metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
+ baseMetricName = "updateSemanticModel")
+ public <E extends Entity & HasIdentifier> SemanticModelEntity
updateSemanticModel(
+ NameIdentifier identifier, Function<E, E> updater) throws IOException {
+ SemanticModelPO oldSemanticModelPO =
getSemanticModelPOByIdentifier(identifier);
+ SemanticModelEntity oldSemanticModelEntity =
+ fromSemanticModelPO(oldSemanticModelPO, identifier.namespace());
+ SemanticModelEntity newEntity = (SemanticModelEntity) updater.apply((E)
oldSemanticModelEntity);
+ Preconditions.checkArgument(
+ Objects.equals(oldSemanticModelEntity.id(), newEntity.id()),
+ "The updated Semantic Model entity id: %s should be same with the
entity id before: %s",
+ newEntity.id(),
+ oldSemanticModelEntity.id());
+
+ AtomicInteger updateResult = new AtomicInteger(-1);
+ try {
+ SemanticModelPO newSemanticModelPO =
updateSemanticModelPO(oldSemanticModelPO, newEntity);
+ String metalakeName = identifier.namespace().level(0);
+ String catalogName = identifier.namespace().level(1);
+ String schemaName = identifier.namespace().level(2);
+ String oldFullName =
+ NameIdentifierUtil.ofSemanticModel(
+ metalakeName, catalogName, schemaName,
oldSemanticModelPO.getSemanticModelName())
+ .toString();
+ boolean isRenamed =
+ !Objects.equals(
+ oldSemanticModelPO.getSemanticModelName(),
newSemanticModelPO.getSemanticModelName());
+
+ SessionUtils.doMultipleWithCommit(
+ // The Semantic Model and its parent were read before this
transaction started. Lock the
+ // observed parent again before either identity or version writes,
so a schema cascade
+ // cannot finish its cleanup and then let this update recreate child
state below it.
+ () ->
+ SchemaMetaService.getInstance()
+ .lockSchemaForEntityWrite(
+ identifier,
+ oldSemanticModelPO.getSchemaId(),
Review Comment:
If the updater changes the entity namespace, `buildSemanticModelPO` resolves
the new schema/catalog IDs, but this transaction locks only the old schema. A
concurrent delete of the target schema can therefore complete before this write
and leave the identity and inserted snapshot under a deleted schema. Handle
namespace changes like the View/Function services by locking the old and target
schemas in catalog-before-schema order, or reject namespace changes explicitly.
--
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]
