apache / apache/gravitino

[Improvement] Managed fileset silently adopts existing directories, causing unintended data deletion on drop

Open
#12,550 0 comments 0 reactions 0 assignees View on GitHub
improvement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.