This is an automated email from the ASF dual-hosted git repository.
mchades pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new fd6638953b [#12550] improvement(fileset): Guard managed fileset
creation on non-empty locations (#13506)
fd6638953b is described below
commit fd6638953b9588f5decc8670ac3235b1a395ac18
Author: Yasu <[email protected]>
AuthorDate: Tue Oct 27 21:53:41 2026 +0900
[#12550] improvement(fileset): Guard managed fileset creation on non-empty
locations (#13506)
### What changes were proposed in this pull request?
Reject creation of a `MANAGED` fileset on an existing non-empty location
unless `allow-existing-location-as-managed=true` is supplied. The
property defaults to `false`, is immutable, and has Java and Python
constants.
Preflight all resolved locations before creating directories, preserving
nested locations within one request. Check directory contents through
`listStatusIterator().hasNext()` and document the explicit adoption
behavior.
### Why are the changes needed?
Silently adopting existing data as managed makes that data subject to
deletion when the fileset is dropped. This adds an explicit choice at
creation time.
Addresses the creation-time protection in #12550. Type conversion is
separate work in #14124.
### Does this PR introduce _any_ user-facing change?
Yes. Existing non-empty locations require `EXTERNAL` or an explicit
managed-adoption override. Existing empty directories, external
filesets, and catalogs with filesystem operations disabled retain their
existing behavior.
Drop and cascade behavior are unchanged. Validation is a creation-time
preflight; it does not reserve paths against concurrent filesystem
writers.
### How was this patch tested?
- `./gradlew :api:build :catalogs:catalog-fileset:build -PskipITs
--max-workers=2 --console=plain`
- Added coverage for inherited and placeholder locations,
case-insensitive true versus false/invalid overrides, immutable-property
validation, and iterator-based emptiness checks. Existing
nested-location and multi-location preflight regression tests remain
covered.
- Java module builds passed: 300 API tests and 277 fileset tests, with
no failures or skips.
- Python fileset API unit test, constant import check, Black formatting
check, and Pylint passed.
Object-store connector behavior was inspected in the pinned dependency
sources; live cloud-storage tests were not run.
---------
Signed-off-by: Yasutaka Hisano <[email protected]>
Co-authored-by: mchades <[email protected]>
---
.../java/org/apache/gravitino/file/Fileset.java | 7 +
.../catalog/fileset/FilesetCatalogOperations.java | 48 ++-
.../catalog/fileset/FilesetPropertiesMetadata.java | 11 +
.../fileset/TestFilesetCatalogOperations.java | 363 +++++++++++++++++++++
.../fileset/TestFilesetPropertiesMetadata.java | 65 ++++
.../client-python/gravitino/api/file/fileset.py | 4 +
docs/fileset-catalog.md | 1 +
docs/filesets.md | 9 +-
docs/manage-fileset-metadata-using-gravitino.md | 4 +
9 files changed, 504 insertions(+), 8 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/file/Fileset.java
b/api/src/main/java/org/apache/gravitino/file/Fileset.java
index 644c516e69..36577de3ed 100644
--- a/api/src/main/java/org/apache/gravitino/file/Fileset.java
+++ b/api/src/main/java/org/apache/gravitino/file/Fileset.java
@@ -71,6 +71,13 @@ public interface Fileset extends Auditable {
/** The property name for the default location name of the fileset. */
String PROPERTY_DEFAULT_LOCATION_NAME = "default-location-name";
+ /**
+ * The property name that allows creating a managed fileset on a location
that already exists and
+ * is not empty. Defaults to false, in which case such a location must be
registered as an
+ * external fileset instead.
+ */
+ String PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED =
"allow-existing-location-as-managed";
+
/** The reserved location name to indicate the location name is unknown. */
String LOCATION_NAME_UNKNOWN = "unknown";
diff --git
a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
index 2f8ef02dfd..af24428c57 100644
---
a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
+++
b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.catalog.fileset;
import static
org.apache.gravitino.catalog.hadoop.fs.HDFSFileSystemProvider.SCHEME_HDFS;
import static org.apache.gravitino.file.Fileset.LOCATION_NAME_UNKNOWN;
+import static
org.apache.gravitino.file.Fileset.PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED;
import static org.apache.gravitino.file.Fileset.PROPERTY_CATALOG_PLACEHOLDER;
import static org.apache.gravitino.file.Fileset.PROPERTY_DEFAULT_LOCATION_NAME;
import static org.apache.gravitino.file.Fileset.PROPERTY_FILESET_PLACEHOLDER;
@@ -542,10 +543,18 @@ public class FilesetCatalogOperations extends
ManagedSchemaOperations
});
} else {
try {
- // formalize the path to avoid path without scheme, uri, authority,
etc.
+ boolean allowExistingLocationAsManaged =
+ (boolean)
+ propertiesMetadata
+ .filesetPropertiesMetadata()
+ .getOrDefault(properties,
PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED);
+ Map<String, Map<String, String>> fileSystemConfigurations =
Maps.newHashMap();
+ // Formalize and validate every location before creating any
directories. Otherwise,
+ // creating a child location can make another location in the request
non-empty.
for (Map.Entry<String, Path> entry : filesetPaths.entrySet()) {
Map<String, String> fsConf =
mergeUpLevelConfigurations(ident, properties, entry.getValue());
+ fileSystemConfigurations.put(entry.getKey(), fsConf);
FileSystem tmpFs = getFileSystemWithCache(entry.getValue(), fsConf);
Path formalizePath =
entry.getValue().makeQualified(tmpFs.getUri(),
tmpFs.getWorkingDirectory());
@@ -553,14 +562,39 @@ public class FilesetCatalogOperations extends
ManagedSchemaOperations
filesetPathsBuilder.put(entry.getKey(), formalizePath);
FileSystem fs = getFileSystemWithCache(formalizePath, fsConf);
- if (fs.exists(formalizePath) &&
fs.getFileStatus(formalizePath).isFile()) {
- throw new IllegalArgumentException(
- "Fileset location cannot be a file: "
- + formalizePath
- + ", location name: "
- + entry.getKey());
+ if (fs.exists(formalizePath)) {
+ if (fs.getFileStatus(formalizePath).isFile()) {
+ throw new IllegalArgumentException(
+ "Fileset location cannot be a file: "
+ + formalizePath
+ + ", location name: "
+ + entry.getKey());
+ }
+ // A managed fileset takes its data with it on drop, so silently
adopting a location
+ // that already holds files would put data Gravitino never created
at risk.
+ if (type == Fileset.Type.MANAGED
+ && !allowExistingLocationAsManaged
+ && fs.listStatusIterator(formalizePath).hasNext()) {
+ throw new IllegalArgumentException(
+ "Location "
+ + formalizePath
+ + " for managed fileset "
+ + ident
+ + " already exists and is not empty, location name: "
+ + entry.getKey()
+ + ". Register it as an "
+ + Fileset.Type.EXTERNAL
+ + " fileset with this location set explicitly, or set "
+ + PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED
+ + "=true to manage it anyway");
+ }
}
+ }
+ for (Map.Entry<String, Path> entry :
filesetPathsBuilder.build().entrySet()) {
+ Path formalizePath = entry.getValue();
+ FileSystem fs =
+ getFileSystemWithCache(formalizePath,
fileSystemConfigurations.get(entry.getKey()));
if (!fs.exists(formalizePath)) {
if (!fs.mkdirs(formalizePath)) {
throw new RuntimeException(
diff --git
a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetPropertiesMetadata.java
b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetPropertiesMetadata.java
index 8f01268adf..69c37ae107 100644
---
a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetPropertiesMetadata.java
+++
b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetPropertiesMetadata.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.catalog.fileset;
import static
org.apache.gravitino.catalog.fileset.FilesetCatalogPropertiesMetadata.AUTHENTICATION_PROPERTY_ENTRIES;
import static
org.apache.gravitino.catalog.fileset.FilesetCatalogPropertiesMetadata.KERBEROS_PROPERTY_ENTRIES;
+import static
org.apache.gravitino.file.Fileset.PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED;
import static org.apache.gravitino.file.Fileset.PROPERTY_CATALOG_PLACEHOLDER;
import static org.apache.gravitino.file.Fileset.PROPERTY_DEFAULT_LOCATION_NAME;
import static org.apache.gravitino.file.Fileset.PROPERTY_FILESET_PLACEHOLDER;
@@ -73,6 +74,16 @@ public class FilesetPropertiesMetadata extends
BasePropertiesMetadata {
true /* immutable */,
null,
false /* hidden */))
+ .put(
+ PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED,
+ PropertyEntry.booleanPropertyEntry(
+ PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED,
+ "Whether a managed fileset may be created on a non-empty
location that already exists",
+ false /* required */,
+ true /* immutable */,
+ false /* default value */,
+ false /* hidden */,
+ false /* reserved */))
.putAll(KERBEROS_PROPERTY_ENTRIES)
.putAll(AUTHENTICATION_PROPERTY_ENTRIES)
.putAll(CloudPropertiesMetadata.STORAGE_PROPERTY_ENTRIES);
diff --git
a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
index 644cf7c9d2..8bfa44dfc6 100644
---
a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
+++
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
@@ -49,6 +49,7 @@ import static
org.apache.gravitino.catalog.fileset.FilesetCatalogPropertiesMetad
import static
org.apache.gravitino.catalog.fileset.FilesetCatalogPropertiesMetadata.LOCATION;
import static
org.apache.gravitino.catalog.hadoop.fs.FileSystemProvider.GRAVITINO_BYPASS;
import static org.apache.gravitino.file.Fileset.LOCATION_NAME_UNKNOWN;
+import static
org.apache.gravitino.file.Fileset.PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED;
import static org.apache.gravitino.file.Fileset.PROPERTY_DEFAULT_LOCATION_NAME;
import static
org.apache.gravitino.file.Fileset.PROPERTY_MULTIPLE_LOCATIONS_PREFIX;
import static org.mockito.ArgumentMatchers.any;
@@ -162,6 +163,7 @@ import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.LocalFileSystem;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
import org.awaitility.Awaitility;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Assertions;
@@ -1508,6 +1510,367 @@ public class TestFilesetCatalogOperations {
}
}
+ /**
+ * Checks that managed filesets reject existing data without registering
metadata.
+ *
+ * @param root temporary directory for this test
+ */
+ @Test
+ public void testCreateManagedFilesetRejectsNonEmptyExistingLocation(@TempDir
File root)
+ throws IOException {
+ String schemaName = "schema_managed_non_empty";
+ String filesetName = "fileset_managed_non_empty";
+ String catalogPath = TEST_ROOT_PATH + "/catalog_managed_non_empty";
+ createSchema(schemaName, "comment", catalogPath, null);
+
+ File existingLocation = new File(root, "existing_reports");
+ FileUtils.forceMkdir(existingLocation);
+ FileUtils.touch(new File(existingLocation, "report.csv"));
+ NameIdentifier filesetIdent = NameIdentifier.of("m1", "c1", schemaName,
filesetName);
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ createFileset(
+ filesetName,
+ schemaName,
+ "comment",
+ Fileset.Type.MANAGED,
+ catalogPath,
+ existingLocation.toURI().toString()));
+ Assertions.assertTrue(exception.getMessage().contains("already exists and
is not empty"));
+
Assertions.assertTrue(exception.getMessage().contains(Fileset.Type.EXTERNAL.name()));
+
+ // Nothing was registered, and the data that would have been at risk is
untouched.
+ try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store,
secretManager)) {
+ ops.initialize(
+ ImmutableMap.of(LOCATION, catalogPath),
+ randomCatalogInfo("m1", "c1"),
+ FILESET_PROPERTIES_METADATA);
+ Assertions.assertThrows(NoSuchFilesetException.class, () ->
ops.loadFileset(filesetIdent));
+ }
+ Assertions.assertTrue(new File(existingLocation, "report.csv").exists());
+ }
+
+ /**
+ * Checks that an empty existing directory can still be adopted by a managed
fileset.
+ *
+ * @param root temporary directory for this test
+ */
+ @Test
+ public void testCreateManagedFilesetAdoptsEmptyExistingLocation(@TempDir
File root)
+ throws IOException {
+ String schemaName = "schema_managed_empty";
+ String filesetName = "fileset_managed_empty";
+ String catalogPath = TEST_ROOT_PATH + "/catalog_managed_empty";
+ createSchema(schemaName, "comment", catalogPath, null);
+
+ File existingLocation = new File(root, "existing_empty");
+ FileUtils.forceMkdir(existingLocation);
+
+ Fileset fileset =
+ createFileset(
+ filesetName,
+ schemaName,
+ "comment",
+ Fileset.Type.MANAGED,
+ catalogPath,
+ existingLocation.toURI().toString());
+ Assertions.assertEquals(Fileset.Type.MANAGED, fileset.type());
+ }
+
+ /**
+ * Checks that explicitly adopted data is deleted when the managed fileset
is dropped.
+ *
+ * @param value the managed-location adoption property value
+ * @param root temporary directory for this test invocation
+ */
+ @ParameterizedTest
+ @ValueSource(strings = {"true", "TRUE", "TrUe"})
+ public void testCreateManagedFilesetOnNonEmptyLocationWithOverride(
+ String value, @TempDir File root) throws IOException {
+ String schemaName = "schema_managed_override_" + value;
+ String filesetName = "fileset_managed_override";
+ String catalogPath = TEST_ROOT_PATH + "/catalog_managed_override";
+ createSchema(schemaName, "comment", catalogPath, null);
+
+ File existingLocation = new File(root, "existing_override");
+ File existingData = new File(existingLocation, "report.csv");
+ FileUtils.forceMkdir(existingLocation);
+ FileUtils.touch(existingData);
+
+ Fileset fileset =
+ createFileset(
+ filesetName,
+ schemaName,
+ "comment",
+ Fileset.Type.MANAGED,
+ catalogPath,
+ existingLocation.toURI().toString(),
+ ImmutableMap.of(PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED,
value));
+ Assertions.assertEquals(Fileset.Type.MANAGED, fileset.type());
+ Assertions.assertTrue(existingData.isFile());
+
+ NameIdentifier filesetIdent = NameIdentifier.of("m1", "c1", schemaName,
filesetName);
+ try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store,
secretManager)) {
+ ops.initialize(
+ ImmutableMap.of(LOCATION, catalogPath),
+ randomCatalogInfo("m1", "c1"),
+ FILESET_PROPERTIES_METADATA);
+ Assertions.assertTrue(ops.dropFileset(filesetIdent));
+ }
+ Assertions.assertFalse(existingData.exists());
+ }
+
+ /** Checks that values other than true do not allow adoption of existing
data. */
+ @ParameterizedTest
+ @ValueSource(strings = {"false", "FALSE", "yes", "1", "", " true "})
+ public void testCreateManagedFilesetRejectsNonTrueOverride(String value)
throws IOException {
+ String schemaName = "schema_managed_invalid_override_" + UUID.randomUUID();
+ createSchema(schemaName, "comment", null, null);
+ File location = new File(UNFORMALIZED_TEST_ROOT_PATH, schemaName);
+ File existingData = new File(location, "report.csv");
+ FileUtils.touch(existingData);
+ NameIdentifier ident = NameIdentifier.of("m1", "c1", schemaName,
"reports");
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ createFileset(
+ ident.name(),
+ schemaName,
+ "comment",
+ Fileset.Type.MANAGED,
+ null,
+ location.toURI().toString(),
+
ImmutableMap.of(PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED, value)));
+ Assertions.assertTrue(exception.getMessage().contains("already exists and
is not empty"));
+ Assertions.assertFalse(store.exists(ident, Entity.EntityType.FILESET));
+ Assertions.assertTrue(existingData.isFile());
+ }
+
+ /** Checks the guard after resolving inherited locations and schema/fileset
placeholders. */
+ @ParameterizedTest
+ @ValueSource(strings = {"catalog", "schema", "placeholder"})
+ public void testCreateManagedFilesetRejectsNonEmptyDerivedLocation(String
source)
+ throws IOException {
+ String schemaName = "schema_managed_derived_" + source;
+ String filesetName = "reports";
+ File root = new File(UNFORMALIZED_TEST_ROOT_PATH, "derived_" + source);
+ String catalogLocation = "catalog".equals(source) ?
root.toURI().toString() : null;
+ String schemaLocation = null;
+ if ("schema".equals(source)) {
+ schemaLocation = new File(root, schemaName).toURI().toString();
+ } else if ("placeholder".equals(source)) {
+ schemaLocation = new Path(root.toURI()).toString() +
"/{{schema}}/{{fileset}}";
+ }
+ createSchema(schemaName, "comment", catalogLocation, schemaLocation);
+ File location = new File(new File(root, schemaName), filesetName);
+ File existingData = new File(location, "report.csv");
+ FileUtils.touch(existingData);
+ NameIdentifier ident = NameIdentifier.of("m1", "c1", schemaName,
filesetName);
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ createFileset(
+ filesetName,
+ schemaName,
+ "comment",
+ Fileset.Type.MANAGED,
+ catalogLocation,
+ null));
+ Assertions.assertTrue(exception.getMessage().contains("already exists and
is not empty"));
+ Assertions.assertFalse(store.exists(ident, Entity.EntityType.FILESET));
+ Assertions.assertTrue(existingData.isFile());
+
+ Fileset adopted =
+ createFileset(
+ filesetName,
+ schemaName,
+ "comment",
+ Fileset.Type.MANAGED,
+ catalogLocation,
+ null,
+ ImmutableMap.of(PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED,
"true"));
+ Assertions.assertEquals(
+ location.toPath(), Paths.get(new
Path(adopted.storageLocation()).toUri()));
+ Assertions.assertTrue(existingData.isFile());
+ }
+
+ /** Checks that the guard only asks the listing iterator whether a child
exists. */
+ @SuppressWarnings("unchecked")
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testCreateManagedFilesetUsesLocationIterator(boolean nonEmpty)
throws IOException {
+ String schemaName = "schema_managed_iterator_" + nonEmpty;
+ createSchema(schemaName, "comment", null, null);
+ NameIdentifier ident = NameIdentifier.of("m1", "c1", schemaName,
"reports");
+ Path location = new Path(TEST_ROOT_PATH + "/iterator_" + nonEmpty);
+ FileSystem fs = Mockito.mock(FileSystem.class);
+ RemoteIterator<FileStatus> iterator = Mockito.mock(RemoteIterator.class);
+ when(fs.getUri()).thenReturn(new Path("file:/").toUri());
+ when(fs.getWorkingDirectory()).thenReturn(new Path("/"));
+ when(fs.exists(location)).thenReturn(true);
+ when(fs.getFileStatus(location)).thenReturn(new FileStatus(0, true, 1, 0,
0, location));
+ when(fs.listStatusIterator(location)).thenReturn(iterator);
+ when(iterator.hasNext()).thenReturn(nonEmpty);
+
+ try (FilesetCatalogOperations ops =
+ Mockito.spy(new FilesetCatalogOperations(store, secretManager))) {
+ ops.initialize(ImmutableMap.of(), randomCatalogInfo("m1", "c1"),
FILESET_PROPERTIES_METADATA);
+ doReturn(fs).when(ops).getFileSystemWithCache(any(Path.class),
any(Map.class));
+ Map<String, String> properties =
+ StringIdentifier.newPropertiesWithId(
+ StringIdentifier.fromId(idGenerator.nextId()),
ImmutableMap.of());
+ if (nonEmpty) {
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.createFileset(
+ ident, "comment", Fileset.Type.MANAGED,
location.toString(), properties));
+ Assertions.assertTrue(exception.getMessage().contains("already exists
and is not empty"));
+ Assertions.assertFalse(store.exists(ident, Entity.EntityType.FILESET));
+ } else {
+ Fileset fileset =
+ ops.createFileset(
+ ident, "comment", Fileset.Type.MANAGED, location.toString(),
properties);
+ Assertions.assertEquals(location, new Path(fileset.storageLocation()));
+ }
+ Mockito.verify(fs, Mockito.never()).listStatus(any(Path.class));
+ Mockito.verify(iterator).hasNext();
+ Mockito.verify(iterator, Mockito.never()).next();
+ Mockito.verify(fs, Mockito.never()).mkdirs(any(Path.class));
+ }
+ }
+
+ /** Checks that creating one location does not invalidate another location
in the same request. */
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testCreateManagedFilesetWithNestedNewLocations(boolean
childFirst)
+ throws IOException {
+ String schemaName = "schema_nested_locations_" + childFirst;
+ createSchema(schemaName, "comment", null, null);
+ File parent = new File(UNFORMALIZED_TEST_ROOT_PATH, "nested_locations_" +
childFirst);
+ File child = new File(parent, "child");
+ Map<String, String> locations =
+ ImmutableMap.of(
+ "a", (childFirst ? child : parent).toURI().toString(),
+ "b", (childFirst ? parent : child).toURI().toString());
+
+ Fileset fileset =
+ createMultiLocationFileset(
+ "nested",
+ schemaName,
+ "comment",
+ Fileset.Type.MANAGED,
+ ImmutableMap.of(),
+ locations,
+ ImmutableMap.of(PROPERTY_DEFAULT_LOCATION_NAME, "a"));
+
+ Assertions.assertTrue(parent.isDirectory());
+ Assertions.assertTrue(child.isDirectory());
+ locations.forEach(
+ (name, location) ->
+ Assertions.assertEquals(
+ new Path(location).toString(),
fileset.storageLocations().get(name)));
+ Assertions.assertTrue(
+ store.exists(
+ NameIdentifier.of("m1", "c1", schemaName, "nested"),
Entity.EntityType.FILESET));
+ }
+
+ /** Checks that a rejected later location leaves both the filesystem and
metadata unchanged. */
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testCreateManagedFilesetPreflightsAllLocations(boolean
existingFile)
+ throws IOException {
+ String schemaName = "schema_preflight_locations_" + existingFile;
+ createSchema(schemaName, "comment", null, null);
+ File newLocation = new File(UNFORMALIZED_TEST_ROOT_PATH, "new_location_" +
existingFile);
+ File existingLocation =
+ new File(UNFORMALIZED_TEST_ROOT_PATH, "existing_location_" +
existingFile);
+ File existingData = existingFile ? existingLocation : new
File(existingLocation, "report.csv");
+ FileUtils.touch(existingData);
+ NameIdentifier ident = NameIdentifier.of("m1", "c1", schemaName,
"preflight");
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ createMultiLocationFileset(
+ ident.name(),
+ schemaName,
+ "comment",
+ Fileset.Type.MANAGED,
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ "a", newLocation.toURI().toString(),
+ "b", existingLocation.toURI().toString()),
+ ImmutableMap.of(PROPERTY_DEFAULT_LOCATION_NAME, "a")));
+
+ Assertions.assertTrue(
+ exception
+ .getMessage()
+ .contains(existingFile ? "cannot be a file" : "already exists and
is not empty"));
+ Assertions.assertFalse(newLocation.exists(), "Preflight must not create
earlier locations");
+ Assertions.assertTrue(existingData.isFile());
+ Assertions.assertFalse(store.exists(ident, Entity.EntityType.FILESET));
+ }
+
+ /** Checks that external filesets can still register existing data and
retain it on drop. */
+ @Test
+ public void testCreateExternalFilesetOnNonEmptyLocation() throws IOException
{
+ String schemaName = "schema_external_non_empty";
+ createSchema(schemaName, "comment", null, null);
+ File location = new File(UNFORMALIZED_TEST_ROOT_PATH, "external_reports");
+ File existingData = new File(location, "report.csv");
+ FileUtils.touch(existingData);
+ NameIdentifier ident = NameIdentifier.of("m1", "c1", schemaName,
"external");
+
+ Fileset fileset =
+ createFileset(
+ ident.name(),
+ schemaName,
+ "comment",
+ Fileset.Type.EXTERNAL,
+ null,
+ location.toURI().toString());
+
+ Assertions.assertEquals(Fileset.Type.EXTERNAL, fileset.type());
+ try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store,
secretManager)) {
+ ops.initialize(ImmutableMap.of(), randomCatalogInfo("m1", "c1"),
FILESET_PROPERTIES_METADATA);
+ Assertions.assertTrue(ops.dropFileset(ident));
+ }
+ Assertions.assertTrue(existingData.isFile());
+ }
+
+ /** Checks that metadata-only catalogs do not inspect or change existing
storage. */
+ @Test
+ public void testCreateManagedFilesetOnNonEmptyLocationWhenFSOpsDisabled()
throws IOException {
+ String schemaName = "schema_managed_non_empty_disabled";
+ createSchema(schemaName, "comment", null, null, true);
+ File location = new File(UNFORMALIZED_TEST_ROOT_PATH,
"managed_reports_disabled");
+ File existingData = new File(location, "report.csv");
+ FileUtils.touch(existingData);
+
+ Fileset fileset =
+ createFileset(
+ "managed",
+ schemaName,
+ "comment",
+ Fileset.Type.MANAGED,
+ null,
+ location.toURI().toString(),
+ true);
+
+ Assertions.assertEquals(Fileset.Type.MANAGED, fileset.type());
+ Assertions.assertTrue(existingData.isFile());
+ }
+
/** Checks independent catalog instances cannot both create the same
fileset. */
@Test
public void testConcurrentCreateAcrossIndependentNodesPreservesWinner()
throws Exception {
diff --git
a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetPropertiesMetadata.java
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetPropertiesMetadata.java
new file mode 100644
index 0000000000..95e76816b5
--- /dev/null
+++
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetPropertiesMetadata.java
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.catalog.fileset;
+
+import static
org.apache.gravitino.file.Fileset.PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.gravitino.catalog.PropertiesMetadataHelpers;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/** Tests the fileset catalog's property validation contract. */
+public class TestFilesetPropertiesMetadata {
+
+ /** Checks that the alter validator rejects both setting and removing the
adoption property. */
+ @Test
+ public void testManagedLocationAdoptionPropertyCannotBeAltered() {
+ FilesetPropertiesMetadata metadata = new FilesetPropertiesMetadata();
+ IllegalArgumentException setFailure =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ PropertiesMetadataHelpers.validatePropertyForAlter(
+ metadata,
+
ImmutableMap.of(PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED, "true"),
+ ImmutableMap.of()));
+ Assertions.assertEquals(
+ "Property "
+ + PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED
+ + " is immutable or reserved, cannot be set",
+ setFailure.getMessage());
+
+ IllegalArgumentException removeFailure =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ PropertiesMetadataHelpers.validatePropertyForAlter(
+ metadata,
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED,
+ PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED)));
+ Assertions.assertEquals(
+ "Property "
+ + PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED
+ + " is immutable or reserved, cannot be deleted",
+ removeFailure.getMessage());
+ }
+}
diff --git a/clients/client-python/gravitino/api/file/fileset.py
b/clients/client-python/gravitino/api/file/fileset.py
index cda622ceaf..5cb75a873c 100644
--- a/clients/client-python/gravitino/api/file/fileset.py
+++ b/clients/client-python/gravitino/api/file/fileset.py
@@ -61,6 +61,10 @@ class Fileset(Auditable):
"""The reserved property name for the fileset name placeholder, when
creating a fileset, all
placeholders as {{fileset}} will be replaced by the fileset name"""
+ PROPERTY_ALLOW_EXISTING_LOCATION_AS_MANAGED =
"allow-existing-location-as-managed"
+ """Whether a managed fileset may adopt an existing non-empty location.
Defaults to false
+ and cannot be changed after creation."""
+
class Type(Enum):
"""An enum representing the type of the fileset object."""
diff --git a/docs/fileset-catalog.md b/docs/fileset-catalog.md
index caf1fc8847..e42b474285 100644
--- a/docs/fileset-catalog.md
+++ b/docs/fileset-catalog.md
@@ -206,6 +206,7 @@ Besides, the Fileset catalog fileset has the following
properties:
| `credential-providers` | The credential provider types,
separated by comma.
| (none)
| No
| No |
| `placeholder-` | Properties that start with
`placeholder-` are used to replace placeholders in the location.
| (none)
| No
| Yes |
| `default-location-name` | The name of the default location of
the fileset, mainly used for GVFS operations without specifying a location
name. | When the fileset has only one location, its location name will be
automatically selected as the default value. | Yes, if the fileset has multiple
locations | Yes |
+| `allow-existing-location-as-managed` | Whether a managed fileset may be
created on a location that already exists and is not empty. Such a location is
rejected by default when filesystem operations are enabled; register it as an
`EXTERNAL` fileset instead. | false
| No | Yes
|
| `config.resources` | The configuration resources,
separated by comma. For example, `hdfs-site.xml,core-site.xml`.
| (none)
| No
| NO |
Some properties are reserved and cannot be set by users:
diff --git a/docs/filesets.md b/docs/filesets.md
index bc48661371..f286eca867 100644
--- a/docs/filesets.md
+++ b/docs/filesets.md
@@ -47,6 +47,8 @@ than federating an external one, so no provider is needed
when creating it.
**2. Create the fileset.** Give it a name, a type, and a storage location.
Choose `MANAGED` to let
Gravitino manage the data lifecycle, or `EXTERNAL` to retain the data when the
fileset is dropped.
Pointing at an existing path does not automatically make a fileset external.
+An existing empty directory can be adopted as `MANAGED`; when filesystem
operations are enabled,
+adopting an existing non-empty directory requires
`allow-existing-location-as-managed=true`.
**3. Read it by name.** Engines reach the files through GVFS or the client
rather than the raw path.
See [How to use GVFS](./how-to-use-gvfs.md).
@@ -56,12 +58,17 @@ See [How to use GVFS](./how-to-use-gvfs.md).
### Managed and External
A managed fileset belongs to Gravitino. Creating it creates the directory if
needed, and deleting
-it deletes the data.
+it deletes the data. An existing empty directory can also be used. With
filesystem operations enabled,
+creating a managed fileset on an existing non-empty location is rejected by
default. Register such a
+location as an external fileset, or set `allow-existing-location-as-managed`
to `true` to explicitly
+allow Gravitino to manage the existing data and delete it when the fileset is
dropped.
An external fileset leaves data lifecycle management to the user. Deleting it
removes only the
Gravitino record and preserves the files. With filesystem operations enabled,
creating either
type creates the storage directory if it does not exist.
+Choose the type based on who owns the data and whether dropping the fileset
should delete it.
+
An existing fileset can be converted between the two types using
[`FilesetChange.updateType`](./manage-fileset-metadata-using-gravitino.md#change-the-fileset-type).
Conversion changes metadata only and preserves its identity, locations, and
files. Converting to
diff --git a/docs/manage-fileset-metadata-using-gravitino.md
b/docs/manage-fileset-metadata-using-gravitino.md
index 10ab499625..57f5febc41 100755
--- a/docs/manage-fileset-metadata-using-gravitino.md
+++ b/docs/manage-fileset-metadata-using-gravitino.md
@@ -21,6 +21,10 @@ managed and external, how storage locations and placeholders
work, and how GVFS
A fileset needs a name, a type, and a storage location. A `MANAGED` fileset is
created and deleted
with its data; an `EXTERNAL` one points at a location that already exists.
+With filesystem operations enabled, creating a `MANAGED` fileset on an
existing non-empty location
+is rejected by default. Set `allow-existing-location-as-managed=true` at
creation to explicitly adopt
+that data, including deleting it when the fileset is dropped. An existing
empty directory needs no
+override.
<Tabs groupId='language' queryString>
<TabItem value="shell" label="REST">
