terrytlu opened a new issue, #17213:
URL: https://github.com/apache/iceberg/issues/17213
### Apache Iceberg version
1.4.3
### Query engine
Spark
### Please describe the bug 🐞
### Background / Motivation
Apache Iceberg tables are often created with an explicit `LOCATION`, and it
is easy for **two tables to point at the same underlying directory**:
- **Accidental** — a `SHOW CREATE TABLE` DDL is copied to a test/temp table
with only the table name changed, *forgetting to change the `LOCATION`*. (This
is exactly how we hit the problem in production; see Real-world impact.)
- **Intentional but hazardous** — in-place / zero-copy migration, where a
new table (e.g. Iceberg) is registered over an existing data directory while
the old table definition is still kept pointing at the same directory; or an
ad-hoc temp table created with an explicit `LOCATION` that coincides with a
production table.
When orphan-file cleanup runs against one of those tables, it computes
"reachable files" from that single table's metadata and deletes everything else
it can list — including the **other table's data and metadata files**.
The community has previously reported this class of problem — originally as
#4265 ("DeleteOrphanFiles didn't check the owned table, may cause meta
missed"), describing exactly the two-tables-share-a-location → metadata
corruption scenario. #4265 was subsequently linked to #4346 and closed when
#4346 was resolved by PR #4652 (the `prefixMismatchMode` work). However, as
shown below, PR #4652 only resolves **scheme/authority prefix mismatches**
during path normalization — a largely distinct problem from two independent
tables sharing one location. The original #4265 scenario, where the two tables
share the **same** scheme + authority + path, was therefore never actually
protected, which is what this issue re-raises.
### Problem description
For `DeleteOrphanFilesSparkAction`:
- `validFileIdentDS()` collects reachable files **only from the target
table** (`contentFileDS(table)`, `manifestDS(table)`, `manifestListDS(table)`,
and `otherMetadataFileDS(table)` which is `recursive=false` → only the current
metadata.json's `previousFiles()`).
- `actualFileIdentDS()` **recursively lists the whole `table.location()`**
directory.
- `findOrphanFiles()` does a left-outer join and deletes anything
listed-but-not-valid.
When a second table shares the same location, all of that second table's
files are "listed but not valid" → deleted. **Both data files and metadata
files are removed**, corrupting the second table.
Note on where the logic lives: orphan-file deletion is **not** implemented
in `iceberg-core`. The `DeleteOrphanFiles` **interface**
(`org.apache.iceberg.actions`, in `iceberg-api`) and its base
`BaseDeleteOrphanFiles` (in `iceberg-core`) only define the action *contract*;
the actual list-and-delete implementation in this repo is
`DeleteOrphanFilesSparkAction` (`iceberg-spark`). What *is* shared in core are
the **reachability primitives** — `TableMetadata`, `ReachableFileUtil`,
`MetadataTableUtils` — used to compute which files a table references. Every
implementation that reuses those primitives and then deletes the
listed-but-unreferenced files (the Spark action, and downstream systems such as
Amoro's `IcebergTableMaintainer.cleanOrphanFiles`) computes reachability from a
**single** table's metadata, so all of them inherit the same vulnerability.
### Reproduction
```sql
-- Create test db
CREATE DATABASE IF NOT EXISTS spark_catalog.repro_db_v2;
-- Table A at a fixed location
CREATE TABLE spark_catalog.repro_db_v2.a (id BIGINT) USING iceberg LOCATION
'hdfs://HDFS78000003/tmp/iceberg_shared_repro_v2';
INSERT INTO spark_catalog.repro_db_v2.a VALUES (1L), (2L);
-- Table B accidentally (or temporarily) points at the SAME location
CREATE TABLE spark_catalog.repro_db_v2.b (id BIGINT) USING iceberg LOCATION
'hdfs://HDFS78000003/tmp/iceberg_shared_repro_v2';
INSERT INTO spark_catalog.repro_db_v2.b VALUES (3L);
-- Run orphan cleanup on A
CALL spark_catalog.system.remove_orphan_files(
table => 'repro_db_v2.a',
older_than => TIMESTAMP '2099-09-10 12:00:00.000');
-- Result: B's metadata.json / manifest / data files under the shared
-- directory are now deleted -> B is corrupted / unloadable.
SELECT * from spark_catalog.repro_db_v2.b;
```
### Why existing mitigations don't help
**`gc.enabled`** (`TableProperties.GC_ENABLED`, default `true`): this is a
*declarative* switch — if a table sets `gc.enabled=false`, its own GC is
disabled. It only protects the table that set it. If Table A keeps the default
`true` while Table B sets `false`, A's cleanup still deletes B's files (A looks
only at A's own reachability). To be safe you must disable GC on **both**
tables — exactly what users forget when they spin up a temp table on a shared
path. `SnapshotTableSparkAction` sets `gc.enabled=false` on the *target*
automatically, but that convention is not applied to arbitrary co-located
tables.
### Real-world impact
This is not a theoretical edge case. We hit exactly this failure mode in our
production environment:
- An analyst created a **test table by copying the `SHOW CREATE TABLE` DDL
of a production table** and only renamed the table, **forgetting to change the
`LOCATION`** clause. The test table therefore pointed at the production table's
directory.
- Much later, an orphan-file cleanup was run against the **test** table.
Because the test table's reachability set does not include the production
table's files, the cleanup deleted the production table's **data and metadata
files** — silent, irreversible data loss on the production table.
- After the incident we scanned the **entire environment** and found **100+
tables** that share a `LOCATION` with another table in the same way. The
proportion is small, but the pattern occurs **unconsciously** (a copy-paste of
DDL) and leads to severe, unrecoverable consequences.
### Proposed solution: automatic location-conflict detection via `table-uuid`
Unlike the `gc.enabled` approach, we propose a **default-on, automatic
location-conflict guard** rather than a convention that users must remember to
follow.
Iceberg already assigns each table an immutable `table-uuid` at creation
time (stored in every `metadata.json`, copied verbatim on each commit; see
`TableMetadata`). If a `metadata/` directory contains a `metadata.json` whose
`table-uuid` differs from the table currently being cleaned, it is strong
evidence that **another table shares this location**, and cleanup should be
aborted (or at least skipped) for safety.
**Design**
Before performing deletions, any `DeleteOrphanFiles` implementation (e.g.
`DeleteOrphanFilesSparkAction`) should:
1. List `metadata/*.metadata.json` (+ `.metadata.json.gz`) in
`table.location()/metadata`.
2. For each file not belonging to the current table's own `previousFiles()`
set, read its `table-uuid`:
- Same `table-uuid` → older version of the current table (truncated by
`previous-versions-max`); ignore.
- Different `table-uuid` → another table shares the location → abort
cleanup (throw a clear exception / skip).
- `table-uuid` unreadable (legacy / compressed / corrupt) → treat as
suspicious → abort/skip (fail-safe).
3. Only proceed when no foreign `table-uuid` is found.
This is automatic (zero config), direction-agnostic (protects both tables
regardless of which one triggers cleanup), and engine-agnostic if placed in the
shared `DeleteOrphanFiles` action contract (e.g. as a guarded step in
`BaseDeleteOrphanFiles`, or a reusable utility in `iceberg-core` that any
engine / downstream system can call before deleting orphans).
**Sketch (Spark action / core)**
```java
private boolean hasOtherTableInLocation(Table table) {
String metadataDir = table.location() + "/metadata/";
String myUuid = ((HasTableOperations) table).operations().current().uuid();
// Files this table already "knows about"
Set<String> myMetadataFiles = new HashSet<>();
TableMetadata current = ((HasTableOperations)
table).operations().current();
myMetadataFiles.add(fileName(current.metadataFileLocation()));
for (MetadataLogEntry e : current.previousFiles()) {
myMetadataFiles.add(fileName(e.file()));
}
PathFilter filter = p -> {
String n = p.getName();
return n.endsWith(".metadata.json") || n.endsWith(".metadata.json.gz");
};
try {
for (FileStatus f : fs.listStatus(new Path(metadataDir), filter)) {
if (myMetadataFiles.contains(f.getPath().getName())) continue;
String otherUuid = readTableUuid(f.getPath()); // gzip-aware; null if
unreadable
if (otherUuid == null) {
LOG.warn("Cannot read table-uuid from {}; treat as suspicious, skip
orphan clean", f.getPath());
return true;
}
if (!otherUuid.equals(myUuid)) {
LOG.warn("Another table (uuid={}) shares location {}; skip orphan
clean for {}",
otherUuid, metadataDir, myUuid);
return true;
}
}
} catch (IOException e) {
LOG.warn("Failed to list metadata dir {}; skip orphan clean",
metadataDir, e);
return true;
}
return false;
}
```
`readTableUuid` must be gzip-aware: detect the gzip magic bytes (`0x1f
0x8b`) and decompress before scanning for `"table-uuid"`.
**Edge cases handled**
- `previous-versions-max` truncation → older versions of the *same* table
share the same `table-uuid`, correctly ignored (only a few extra reads).
- `.metadata.json.gz` → handled by gzip-aware read + matching file filter.
- Legacy files without `table-uuid` → `TableMetadataParser` reads it via
`getStringOrNull`; treated as suspicious (fail-safe skip), never silently
allowed.
- Performance → on the normal (no-conflict) path it is a single `listStatus`
(~10ms) plus zero file reads (all listed files belong to the current table).
### Alternatives considered
- Rely on `gc.enabled=false` on both tables — manual, easy to forget;
doesn't scale to temp/ad-hoc tables.
- Rely on `prefixMismatchMode` — only catches scheme/authority differences,
misses the dominant shared-`hdfs`/`s3` case.
- Require a unique location per table (convention) — can't be enforced by
the engine and breaks legitimate snapshot-table patterns.
### Ask
We'd like to propose adding this location-conflict guard to the
`DeleteOrphanFiles` action contract. Note that the only in-repo implementation
today is `DeleteOrphanFilesSparkAction`; the same pattern must also protect any
engine / downstream system that lists-and-deletes orphans from a single table's
metadata (e.g. Amoro's `IcebergTableMaintainer.cleanOrphanFiles`). Happy to
contribute a PR.
### Willingness to contribute
- [x] I can contribute a fix for this bug independently
- [ ] I would be willing to contribute a fix for this bug with guidance from
the Iceberg community
- [ ] I cannot contribute a fix for this bug at this time
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
