mchades commented on code in PR #14120:
URL: https://github.com/apache/gravitino/pull/14120#discussion_r4347023449
##########
catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java:
##########
@@ -925,71 +895,32 @@ public boolean dropSchema(NameIdentifier ident, boolean
cascade) throws NonEmpty
Optional.ofNullable(schemaEntity.properties()).orElse(Collections.emptyMap());
Map<String, Path> schemaPaths = getAndCheckSchemaPaths(ident.name(),
properties);
- boolean dropped = super.dropSchema(ident, cascade);
- if (disableFSOps) {
- // No FS cleanup; still remove fileset write-through secrets when
cascade deleted them.
- // Schema secrets are cleaned by SchemaOperationDispatcher.
- if (dropped && cascade) {
- for (FilesetEntity fileset : filesets) {
- secretManager.deleteSecretsFromProperties(fileset.properties());
- }
+ // Catalog force-drop reaches this method without
SchemaOperationDispatcher's fileset
+ // cleanup. Drop each fileset before its schema so a storage failure
preserves the failed
+ // fileset and its parent for retry. Stop at the first failure; earlier
drops stay committed.
+ if (cascade) {
+ for (FilesetEntity fileset : filesets) {
+ dropFilesetAndGet(fileset.nameIdentifier())
+ .ifPresent(
+ deleted ->
secretManager.deleteSecretsFromProperties(deleted.properties()));
+ }
+
+ // Functions can also live in fileset catalogs. Drop their metadata
explicitly before
+ // the non-cascading schema delete so it still protects any
unprocessed filesets.
+ for (FunctionEntity function :
+ store.list(filesetNs, FunctionEntity.class,
Entity.EntityType.FUNCTION)) {
+ store.delete(function.nameIdentifier(), Entity.EntityType.FUNCTION);
}
- return dropped;
}
+ // Reject remaining filesets instead of deleting their metadata without
storage cleanup.
+ boolean dropped = super.dropSchema(ident, false);
Review Comment:
Schema directory cleanup now runs before the metadata transaction commits.
Failures retain the schema and its secrets for retry; missing directories are
accepted.
##########
catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java:
##########
@@ -925,71 +895,32 @@ public boolean dropSchema(NameIdentifier ident, boolean
cascade) throws NonEmpty
Optional.ofNullable(schemaEntity.properties()).orElse(Collections.emptyMap());
Map<String, Path> schemaPaths = getAndCheckSchemaPaths(ident.name(),
properties);
- boolean dropped = super.dropSchema(ident, cascade);
- if (disableFSOps) {
- // No FS cleanup; still remove fileset write-through secrets when
cascade deleted them.
- // Schema secrets are cleaned by SchemaOperationDispatcher.
- if (dropped && cascade) {
- for (FilesetEntity fileset : filesets) {
- secretManager.deleteSecretsFromProperties(fileset.properties());
- }
+ // Catalog force-drop reaches this method without
SchemaOperationDispatcher's fileset
+ // cleanup. Drop each fileset before its schema so a storage failure
preserves the failed
+ // fileset and its parent for retry. Stop at the first failure; earlier
drops stay committed.
+ if (cascade) {
+ for (FilesetEntity fileset : filesets) {
+ dropFilesetAndGet(fileset.nameIdentifier())
+ .ifPresent(
+ deleted ->
secretManager.deleteSecretsFromProperties(deleted.properties()));
+ }
+
+ // Functions can also live in fileset catalogs. Drop their metadata
explicitly before
Review Comment:
Function cleanup and schema deletion now share one metadata transaction. The
regression for concurrent fileset creation verifies that the function survives
a rejected drop and is removed on retry.
--
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]
