[Improvement] Managed fileset silently adopts existing directories, causing unintended data deletion on drop
- Dominant language
- Java
- Stars
- 3.2k
- Forks
- 935
- Avg merge
- 1d 17h
- Merged PRs (30d)
- 339
Description
### What would you like to be improved?
Currently, when creating a managed fileset that points to an already-existing directory, Gravitino
silently "adopts" that directory without any warning or rejection. Later, when the fileset (or its
parent schema/catalog) is dropped, the physical directory is recursively deleted because it is
managed(default).
This creates a serious data-loss issue in the following scenario:
1. User creates a managed fileset pointing to an existing path (e.g., `/data/production/reports`)
— Gravitino logs `"Fileset {} manages the existing location {}"` but proceeds without error.
2. User later realizes the fileset was misconfigured and drops it, or drops the catalog with
`force=true`.
3. The entire `/data/production/reports` directory is recursively deleted.
Relevant code:
- `FilesetCatalogOperations.createMultipleLocationFileset()`: when the path already exists, it only
logs an INFO message and continues (no rejection, no type enforcement).
- `FilesetCatalogOperations.dropFileset()`: unconditionally calls `fs.delete(location, true)` for
MANAGED filesets when `disableFSOps` is false.
There is no fileset-level property to prevent physical deletion on drop. The only available controls
are:
- Choosing `EXTERNAL` type at creation time (but users may not be aware of the consequence of
MANAGED being the default).
- Setting `disable-filesystem-ops=true` at catalog level (which disables ALL filesystem operations,
not just delete-on-drop).
### How should we improve?
Proposed improvements (in priority order):
1. **Reject or warn when creating a MANAGED fileset on an existing path**
- Option A (safer): If the storage path already exists, reject creating it as MANAGED and require
the user to explicitly use EXTERNAL type, or add a property like
`"allow-existing-location-as-managed": "true"` to override.
- Option B (less disruptive): Log a WARN and add documentation, but still allow it.
2. **Add a fileset-level `delete-data-on-drop` property**
- Default to `true` for MANAGED filesets (preserving current behavior).
- Allow users to set it to `false` on a per-fileset basis to protect data even for MANAGED type.
- This provides a safety net for cases where users want Gravitino to manage the lifecycle
metadata but not the physical deletion.
3. **Documentation improvement**
- Clearly document in the fileset creation API/docs that MANAGED type will delete physical data
on drop, and that pointing a managed fileset at an existing directory means that directory
will be deleted when the fileset is dropped.
Contributor guide
Research direction
Start with FilesetCatalogOperations.createMultipleLocationFileset() and dropFileset(), tracing the existing-location handling and MANAGED deletion path, then review the fileset creation API and documentation. Done requires a maintainer-approved choice among rejection, warning, deletion control, and documentation changes, with the existing-path and drop behavior verified.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, data-engineering
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100