diff --git a/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java b/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java index 9bfa440e083..b6c44f4ac3e 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java @@ -130,56 +130,53 @@ public void setOwner( public Optional getOwner(String metalake, MetadataObject metadataObject) { NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake, metadataObject); + OwnerImpl owner = new OwnerImpl(); + try { + List entities = + TreeLockUtils.doWithTreeLock( + ident, + LockType.READ, + () -> + store + .relationOperations() + .listEntitiesByRelation( + SupportsRelationOperations.Type.OWNER_REL, + ident, + MetadataObjectUtil.toEntityType(metadataObject))); + + if (entities.isEmpty()) { + return Optional.empty(); + } + + if (entities.size() != 1) { + throw new IllegalStateException( + String.format("The number of the owner %s must be 1", metadataObject.fullName())); + } + + Entity entity = entities.get(0); + if (!(entity instanceof UserEntity) && !(entity instanceof GroupEntity)) { + throw new IllegalArgumentException( + String.format( + "Doesn't support owner entity class %s", entities.get(0).getClass().getName())); + } - return TreeLockUtils.doWithTreeLock( - ident, - LockType.READ, - () -> { - try { - OwnerImpl owner = new OwnerImpl(); - List entities = - store - .relationOperations() - .listEntitiesByRelation( - SupportsRelationOperations.Type.OWNER_REL, - ident, - MetadataObjectUtil.toEntityType(metadataObject)); - - if (entities.isEmpty()) { - return Optional.empty(); - } - - if (entities.size() != 1) { - throw new IllegalStateException( - String.format("The number of the owner %s must be 1", metadataObject.fullName())); - } - - Entity entity = entities.get(0); - if (!(entity instanceof UserEntity) && !(entity instanceof GroupEntity)) { - throw new IllegalArgumentException( - String.format( - "Doesn't support owner entity class %s", - entities.get(0).getClass().getName())); - } - - if (entities.get(0) instanceof UserEntity) { - UserEntity user = (UserEntity) entities.get(0); - owner.name = user.name(); - owner.type = Owner.Type.USER; - } else if (entities.get(0) instanceof GroupEntity) { - GroupEntity group = (GroupEntity) entities.get(0); - owner.name = group.name(); - owner.type = Owner.Type.GROUP; - } - return Optional.of(owner); - } catch (NoSuchEntityException nse) { - throw new NoSuchMetadataObjectException( - "The metadata object of %s isn't found", metadataObject.fullName()); - } catch (IOException ioe) { - LOG.info("Fail to get the owner of entity {}", metadataObject.fullName(), ioe); - throw new RuntimeException(ioe); - } - }); + if (entities.get(0) instanceof UserEntity) { + UserEntity user = (UserEntity) entities.get(0); + owner.name = user.name(); + owner.type = Owner.Type.USER; + } else if (entities.get(0) instanceof GroupEntity) { + GroupEntity group = (GroupEntity) entities.get(0); + owner.name = group.name(); + owner.type = Owner.Type.GROUP; + } + return Optional.of(owner); + } catch (NoSuchEntityException nse) { + throw new NoSuchMetadataObjectException( + "The metadata object of %s isn't found", metadataObject.fullName()); + } catch (IOException ioe) { + LOG.info("Fail to get the owner of entity {}", metadataObject.fullName(), ioe); + throw new RuntimeException(ioe); + } } private static class OwnerImpl implements Owner { diff --git a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java index 4beb79dc8da..631b3f4e2c0 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java +++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java @@ -340,24 +340,22 @@ public NameIdentifier[] listCatalogs(Namespace namespace) throws NoSuchMetalakeE @Override public Catalog[] listCatalogsInfo(Namespace namespace) throws NoSuchMetalakeException { NameIdentifier metalakeIdent = NameIdentifier.of(namespace.levels()); - return TreeLockUtils.doWithTreeLock( - metalakeIdent, - LockType.READ, - () -> { - checkMetalake(metalakeIdent, store); - try { - List catalogEntities = - store.list(namespace, CatalogEntity.class, EntityType.CATALOG); - - return catalogEntities.stream() - .map(e -> e.toCatalogInfoWithResolvedProps(getResolvedProperties(e))) - .toArray(Catalog[]::new); - - } catch (IOException ioe) { - LOG.error("Failed to list catalogs in metalake {}", metalakeIdent, ioe); - throw new RuntimeException(ioe); - } - }); + try { + List catalogEntities = + TreeLockUtils.doWithTreeLock( + metalakeIdent, + LockType.READ, + () -> { + checkMetalake(metalakeIdent, store); + return store.list(namespace, CatalogEntity.class, EntityType.CATALOG); + }); + return catalogEntities.stream() + .map(e -> e.toCatalogInfoWithResolvedProps(getResolvedProperties(e))) + .toArray(Catalog[]::new); + } catch (IOException ioe) { + LOG.error("Failed to list catalogs in metalake {}", metalakeIdent, ioe); + throw new RuntimeException(ioe); + } } /** @@ -370,7 +368,6 @@ public Catalog[] listCatalogsInfo(Namespace namespace) throws NoSuchMetalakeExce @Override public Catalog loadCatalog(NameIdentifier ident) throws NoSuchCatalogException { NameIdentifier metalakeIdent = NameIdentifier.of(ident.namespace().levels()); - return TreeLockUtils.doWithTreeLock( ident, LockType.READ, @@ -402,35 +399,34 @@ public Catalog createCatalog( throws NoSuchMetalakeException, CatalogAlreadyExistsException { NameIdentifier metalakeIdent = NameIdentifier.of(ident.namespace().levels()); + Map mergedConfig = buildCatalogConf(provider, properties); + long uid = idGenerator.nextId(); + StringIdentifier stringId = StringIdentifier.fromId(uid); + Instant now = Instant.now(); + String creator = PrincipalUtils.getCurrentPrincipal().getName(); + CatalogEntity e = + CatalogEntity.builder() + .withId(uid) + .withName(ident.name()) + .withNamespace(ident.namespace()) + .withType(type) + .withProvider(provider) + .withComment(comment) + .withProperties(StringIdentifier.newPropertiesWithId(stringId, mergedConfig)) + .withAuditInfo( + AuditInfo.builder() + .withCreator(creator) + .withCreateTime(now) + .withLastModifier(creator) + .withLastModifiedTime(now) + .build()) + .build(); + return TreeLockUtils.doWithTreeLock( metalakeIdent, LockType.WRITE, () -> { checkMetalake(metalakeIdent, store); - Map mergedConfig = buildCatalogConf(provider, properties); - - long uid = idGenerator.nextId(); - StringIdentifier stringId = StringIdentifier.fromId(uid); - Instant now = Instant.now(); - String creator = PrincipalUtils.getCurrentPrincipal().getName(); - CatalogEntity e = - CatalogEntity.builder() - .withId(uid) - .withName(ident.name()) - .withNamespace(ident.namespace()) - .withType(type) - .withProvider(provider) - .withComment(comment) - .withProperties(StringIdentifier.newPropertiesWithId(stringId, mergedConfig)) - .withAuditInfo( - AuditInfo.builder() - .withCreator(creator) - .withCreateTime(now) - .withLastModifier(creator) - .withLastModifiedTime(now) - .build()) - .build(); - boolean needClean = true; try { store.put(e, false /* overwrite */); diff --git a/core/src/main/java/org/apache/gravitino/catalog/FilesetOperationDispatcher.java b/core/src/main/java/org/apache/gravitino/catalog/FilesetOperationDispatcher.java index b49ffbdb149..7f37c8da0c4 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/FilesetOperationDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/catalog/FilesetOperationDispatcher.java @@ -79,23 +79,22 @@ public NameIdentifier[] listFilesets(Namespace namespace) throws NoSuchSchemaExc @Override public Fileset loadFileset(NameIdentifier ident) throws NoSuchFilesetException { NameIdentifier catalogIdent = getCatalogIdentifier(ident); - return TreeLockUtils.doWithTreeLock( - ident, - LockType.READ, - () -> { - Fileset fileset = - doWithCatalog( - catalogIdent, - c -> c.doWithFilesetOps(f -> f.loadFileset(ident)), - NoSuchFilesetException.class); - // Currently we only support maintaining the Fileset in the Gravitino's store. - return EntityCombinedFileset.of(fileset) - .withHiddenProperties( - getHiddenPropertyNames( - catalogIdent, - HasPropertyMetadata::filesetPropertiesMetadata, - fileset.properties())); - }); + Fileset fileset = + TreeLockUtils.doWithTreeLock( + ident, + LockType.READ, + () -> + doWithCatalog( + catalogIdent, + c -> c.doWithFilesetOps(f -> f.loadFileset(ident)), + NoSuchFilesetException.class)); + // Currently we only support maintaining the Fileset in the Gravitino's store. + return EntityCombinedFileset.of(fileset) + .withHiddenProperties( + getHiddenPropertyNames( + catalogIdent, + HasPropertyMetadata::filesetPropertiesMetadata, + fileset.properties())); } /** @@ -124,42 +123,40 @@ public Fileset createFileset( Map properties) throws NoSuchSchemaException, FilesetAlreadyExistsException { NameIdentifier catalogIdent = getCatalogIdentifier(ident); + doWithCatalog( + catalogIdent, + c -> + c.doWithPropertiesMeta( + p -> { + validatePropertyForCreate(p.filesetPropertiesMetadata(), properties); + return null; + }), + IllegalArgumentException.class); + long uid = idGenerator.nextId(); + StringIdentifier stringId = StringIdentifier.fromId(uid); + Map updatedProperties = + StringIdentifier.newPropertiesWithId(stringId, properties); - return TreeLockUtils.doWithTreeLock( - NameIdentifier.of(ident.namespace().levels()), - LockType.WRITE, - () -> { - doWithCatalog( - catalogIdent, - c -> - c.doWithPropertiesMeta( - p -> { - validatePropertyForCreate(p.filesetPropertiesMetadata(), properties); - return null; - }), - IllegalArgumentException.class); - long uid = idGenerator.nextId(); - StringIdentifier stringId = StringIdentifier.fromId(uid); - Map updatedProperties = - StringIdentifier.newPropertiesWithId(stringId, properties); - - Fileset createdFileset = - doWithCatalog( - catalogIdent, - c -> - c.doWithFilesetOps( - f -> - f.createFileset( - ident, comment, type, storageLocation, updatedProperties)), - NoSuchSchemaException.class, - FilesetAlreadyExistsException.class); - return EntityCombinedFileset.of(createdFileset) - .withHiddenProperties( - getHiddenPropertyNames( - catalogIdent, - HasPropertyMetadata::filesetPropertiesMetadata, - createdFileset.properties())); - }); + Fileset createdFileset = + TreeLockUtils.doWithTreeLock( + NameIdentifier.of(ident.namespace().levels()), + LockType.WRITE, + () -> + doWithCatalog( + catalogIdent, + c -> + c.doWithFilesetOps( + f -> + f.createFileset( + ident, comment, type, storageLocation, updatedProperties)), + NoSuchSchemaException.class, + FilesetAlreadyExistsException.class)); + return EntityCombinedFileset.of(createdFileset) + .withHiddenProperties( + getHiddenPropertyNames( + catalogIdent, + HasPropertyMetadata::filesetPropertiesMetadata, + createdFileset.properties())); } /** @@ -180,26 +177,26 @@ public Fileset createFileset( @Override public Fileset alterFileset(NameIdentifier ident, FilesetChange... changes) throws NoSuchFilesetException, IllegalArgumentException { - return TreeLockUtils.doWithTreeLock( - NameIdentifier.of(ident.namespace().levels()), - LockType.WRITE, - () -> { - validateAlterProperties(ident, HasPropertyMetadata::filesetPropertiesMetadata, changes); + validateAlterProperties(ident, HasPropertyMetadata::filesetPropertiesMetadata, changes); + NameIdentifier catalogIdent = getCatalogIdentifier(ident); + + Fileset alteredFileset = + TreeLockUtils.doWithTreeLock( + NameIdentifier.of(ident.namespace().levels()), + LockType.WRITE, + () -> + doWithCatalog( + catalogIdent, + c -> c.doWithFilesetOps(f -> f.alterFileset(ident, changes)), + NoSuchFilesetException.class, + IllegalArgumentException.class)); - NameIdentifier catalogIdent = getCatalogIdentifier(ident); - Fileset alteredFileset = - doWithCatalog( - catalogIdent, - c -> c.doWithFilesetOps(f -> f.alterFileset(ident, changes)), - NoSuchFilesetException.class, - IllegalArgumentException.class); - return EntityCombinedFileset.of(alteredFileset) - .withHiddenProperties( - getHiddenPropertyNames( - catalogIdent, - HasPropertyMetadata::filesetPropertiesMetadata, - alteredFileset.properties())); - }); + return EntityCombinedFileset.of(alteredFileset) + .withHiddenProperties( + getHiddenPropertyNames( + catalogIdent, + HasPropertyMetadata::filesetPropertiesMetadata, + alteredFileset.properties())); } /** diff --git a/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java b/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java index 93ee3130b99..d453f5db20c 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java @@ -97,29 +97,29 @@ public NameIdentifier[] listSchemas(Namespace namespace) throws NoSuchCatalogExc @Override public Schema createSchema(NameIdentifier ident, String comment, Map properties) throws NoSuchCatalogException, SchemaAlreadyExistsException { - NameIdentifier catalogIdent = getCatalogIdentifier(ident); + + doWithCatalog( + catalogIdent, + c -> + c.doWithPropertiesMeta( + p -> { + validatePropertyForCreate(p.schemaPropertiesMetadata(), properties); + return null; + }), + IllegalArgumentException.class); + long uid = idGenerator.nextId(); + // Add StringIdentifier to the properties, the specific catalog will handle this + // StringIdentifier to make sure only when the operation is successful, the related + // SchemaEntity will be visible. + StringIdentifier stringId = StringIdentifier.fromId(uid); + Map updatedProperties = + StringIdentifier.newPropertiesWithId(stringId, properties); + return TreeLockUtils.doWithTreeLock( catalogIdent, LockType.WRITE, () -> { - doWithCatalog( - catalogIdent, - c -> - c.doWithPropertiesMeta( - p -> { - validatePropertyForCreate(p.schemaPropertiesMetadata(), properties); - return null; - }), - IllegalArgumentException.class); - long uid = idGenerator.nextId(); - // Add StringIdentifier to the properties, the specific catalog will handle this - // StringIdentifier to make sure only when the operation is successful, the related - // SchemaEntity will be visible. - StringIdentifier stringId = StringIdentifier.fromId(uid); - Map updatedProperties = - StringIdentifier.newPropertiesWithId(stringId, properties); - // we do not retrieve the schema again (to obtain some values generated by underlying // catalog) // since some catalogs' API is async and the schema may not be created immediately diff --git a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java index fee7c1bdc1c..3c84b91d3ac 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java @@ -197,11 +197,11 @@ public Table createTable( @Override public Table alterTable(NameIdentifier ident, TableChange... changes) throws NoSuchTableException, IllegalArgumentException { + validateAlterProperties(ident, HasPropertyMetadata::tablePropertiesMetadata, changes); return TreeLockUtils.doWithTreeLock( NameIdentifier.of(ident.namespace().levels()), LockType.WRITE, () -> { - validateAlterProperties(ident, HasPropertyMetadata::tablePropertiesMetadata, changes); NameIdentifier catalogIdent = getCatalogIdentifier(ident); Table alteredTable = doWithCatalog( @@ -333,11 +333,11 @@ public boolean dropTable(NameIdentifier ident) { @Override public boolean purgeTable(NameIdentifier ident) throws UnsupportedOperationException { NameIdentifier schemaIdentifier = NameIdentifierUtil.getSchemaIdentifier(ident); + NameIdentifier catalogIdent = getCatalogIdentifier(ident); return TreeLockUtils.doWithTreeLock( schemaIdentifier, LockType.WRITE, () -> { - NameIdentifier catalogIdent = getCatalogIdentifier(ident); boolean droppedFromCatalog = doWithCatalog( catalogIdent, @@ -355,7 +355,7 @@ public boolean purgeTable(NameIdentifier ident) throws UnsupportedOperationExcep // catalog into account. // // For managed table, we should take the return value of the store operation into account. - boolean droppedFromStore = false; + boolean droppedFromStore; try { droppedFromStore = store.delete(ident, TABLE); } catch (NoSuchEntityException e) { diff --git a/core/src/main/java/org/apache/gravitino/catalog/TopicOperationDispatcher.java b/core/src/main/java/org/apache/gravitino/catalog/TopicOperationDispatcher.java index 3042cb20663..a04ad3c7733 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/TopicOperationDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/catalog/TopicOperationDispatcher.java @@ -157,12 +157,12 @@ public Topic createTopic( public Topic alterTopic(NameIdentifier ident, TopicChange... changes) throws NoSuchTopicException, IllegalArgumentException { + validateAlterProperties(ident, HasPropertyMetadata::topicPropertiesMetadata, changes); + return TreeLockUtils.doWithTreeLock( NameIdentifier.of(ident.namespace().levels()), LockType.WRITE, () -> { - validateAlterProperties(ident, HasPropertyMetadata::topicPropertiesMetadata, changes); - NameIdentifier catalogIdent = getCatalogIdentifier(ident); // we do not retrieve the topic again (to obtain some values generated by underlying diff --git a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java index 219ca7bdac4..56e2c6baebf 100644 --- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java +++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java @@ -123,20 +123,20 @@ public static boolean metalakeInUse(EntityStore store, NameIdentifier ident) */ @Override public BaseMetalake[] listMetalakes() { - return TreeLockUtils.doWithRootTreeLock( - LockType.READ, - () -> { - try { - return store.list(Namespace.empty(), BaseMetalake.class, EntityType.METALAKE).stream() - .map(this::newMetalakeWithResolvedProperties) - .toArray(BaseMetalake[]::new); - } catch (IOException ioe) { - LOG.error("Listing Metalakes failed due to storage issues.", ioe); - throw new RuntimeException(ioe); - } - }); + try { + List metalakes = + TreeLockUtils.doWithRootTreeLock( + LockType.READ, + () -> store.list(Namespace.empty(), BaseMetalake.class, EntityType.METALAKE)); + + return metalakes.stream() + .map(this::newMetalakeWithResolvedProperties) + .toArray(BaseMetalake[]::new); + } catch (IOException ioe) { + LOG.error("Listing Metalakes failed due to storage issues", ioe); + throw new RuntimeException(ioe); + } } - /** * Loads a Metalake. * @@ -148,8 +148,12 @@ public BaseMetalake[] listMetalakes() { @Override public BaseMetalake loadMetalake(NameIdentifier ident) throws NoSuchMetalakeException { try { - return newMetalakeWithResolvedProperties( - store.get(ident, EntityType.METALAKE, BaseMetalake.class)); + BaseMetalake baseMetalake = + TreeLockUtils.doWithTreeLock( + ident, + LockType.READ, + () -> store.get(ident, EntityType.METALAKE, BaseMetalake.class)); + return newMetalakeWithResolvedProperties(baseMetalake); } catch (NoSuchEntityException e) { LOG.warn("Metalake {} does not exist", ident, e); throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, ident); diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java index f91b09f92f1..827c7a0b9d1 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java @@ -51,8 +51,6 @@ import org.apache.gravitino.dto.responses.MetalakeListResponse; import org.apache.gravitino.dto.responses.MetalakeResponse; import org.apache.gravitino.dto.util.DTOConverters; -import org.apache.gravitino.lock.LockType; -import org.apache.gravitino.lock.TreeLockUtils; import org.apache.gravitino.metalake.MetalakeDispatcher; import org.apache.gravitino.metrics.MetricNames; import org.apache.gravitino.server.web.Utils; @@ -137,9 +135,7 @@ public Response loadMetalake(@PathParam("name") String metalakeName) { httpRequest, () -> { NameIdentifier identifier = NameIdentifierUtil.ofMetalake(metalakeName); - Metalake metalake = - TreeLockUtils.doWithTreeLock( - identifier, LockType.READ, () -> metalakeDispatcher.loadMetalake(identifier)); + Metalake metalake = metalakeDispatcher.loadMetalake(identifier); Response response = Utils.ok(new MetalakeResponse(DTOConverters.toDTO(metalake))); LOG.info("Metalake loaded: {}", metalake.name()); return response;