Skip to content

[WIP][AMORO-4292][AMS] Guard IcebergTableMaintainer orphan cleanup against shared table locations - #4293

Open
terrytlu wants to merge 2 commits into
apache:masterfrom
terrytlu:AMORO-4292-orphan-clean-location-conflict
Open

[WIP][AMORO-4292][AMS] Guard IcebergTableMaintainer orphan cleanup against shared table locations#4293
terrytlu wants to merge 2 commits into
apache:masterfrom
terrytlu:AMORO-4292-orphan-clean-location-conflict

Conversation

@terrytlu

@terrytlu terrytlu commented Jul 27, 2026

Copy link
Copy Markdown

Fixes #4292

What changes were proposed in this pull request?

IcebergTableMaintainer.cleanOrphanFiles() lists and deletes files under a table's location. When two Iceberg tables share the same location (e.g. one table is created from another's metadata directory), the cleanup of table A can delete files that actually belong to table B, silently corrupting B.

This PR adds a pre-check, IcebergTableUtil.hasOtherTableInLocation(Table):

  • lists the table's metadata directory via its own (storage-agnostic) FileIO (requires SupportsPrefixOperations; otherwise a ValidationException is thrown so the caller can decide);
  • compares the table-uuid of every other metadata.json with the current table's uuid;
  • an unreadable / legacy / compressed metadata file (uuid cannot be determined) is treated as a conflict (fail-safe).

If another table is detected in the same location, cleanOrphanFiles() now skips cleanup by default (fail-closed) instead of risking another table's files. A new table property clean-orphan-file.ignore-location-conflict (default false) lets users opt out of the check when they are certain the location is not shared.

Why are the changes needed?

Without the guard, a single misconfigured / duplicated table location leads to silent cross-table data loss during scheduled orphan-file cleanup.

How was this patch tested?

  • Added TestIcebergOrphanFileLocationConflict (5 cases × 2 Iceberg parameterizations = 10 tests): no conflict, conflict via corrupt metadata, ValidationException when FileIO lacks prefix support, default skip-on-conflict, and ignore-conflict-when-property-enabled proceeds with cleanup.
  • mvn test -pl amoro-ams -Dtest=TestIcebergOrphanFileLocationConflict → Tests run: 10, Failures: 0, Errors: 0.
  • spotless:check passes.

Documentation

This PR introduces a new table property clean-orphan-file.ignore-location-conflict (default false), which allows users to bypass the shared-location check when they are certain the table location is not shared. The property is documented in docs/user-guides/configurations.md (Self-optimizing / table maintenance configurations section).

@github-actions github-actions Bot added type:docs Improvements or additions to documentation module:ams-server Ams server module module:common labels Jul 27, 2026
@terrytlu terrytlu changed the title [WIP][AMORO-4292][AMS] Guard IcebergTableMaintainer orphan cleanup against shared table locations [AMORO-4292][AMS] Guard IcebergTableMaintainer orphan cleanup against shared table locations Jul 27, 2026
…ed table locations

IcebergTableMaintainer.cleanOrphanFiles() lists and deletes files under a
table's location. When two Iceberg tables share the same location (for
example, one table is created from another's metadata directory, or a
snapshot is restored into the same path), the cleanup of table A can delete
files that actually belong to table B, silently corrupting B. This is the
same root cause reported for Iceberg in apache/iceberg#17213.

This change adds a pre-check, IcebergTableUtil.hasOtherTableInLocation(Table),
that lists the table's metadata directory via its own (storage-agnostic)
FileIO and compares the table-uuid of every other metadata.json with the
current table's uuid. If another table is detected in the same location,
cleanOrphanFiles() now skips cleanup by default (fail-closed) instead of
risking another table's files. A new table property,
clean-orphan-file.ignore-location-conflict (default false), lets users opt
out of the check when they are certain the location is not shared.

Fixes apache#4292

Signed-off-by: terrytlu <terrytlu@tencent.com>
@terrytlu
terrytlu force-pushed the AMORO-4292-orphan-clean-location-conflict branch 2 times, most recently from 947fb94 to bd9a4b6 Compare August 7, 2026 10:42
@terrytlu terrytlu changed the title [AMORO-4292][AMS] Guard IcebergTableMaintainer orphan cleanup against shared table locations [WIP][AMORO-4292][AMS] Guard IcebergTableMaintainer orphan cleanup against shared table locations Aug 7, 2026
…ed table locations, add metrics monitoring orphan file cleaning status

Introduce last-value gauges to track orphan-file-cleaning outcomes:
- table_orphan_file_cleaning_last_status (0=SUCCESS, 1=CONFLICT_DETECTED,
  2=CHECK_UNAVAILABLE, 3=EXECUTION_FAILED)
- table_orphan_file_cleaning_last_failure_timestamp_ms

MaintainerMetrics gains CleanFailureReason and recordSuccess/recordFailure.
IcebergTableMaintainer wraps cleanup in try/catch and records success/failure,
with location-conflict check returning CONFLICT_DETECTED/CHECK_FAILED.
@terrytlu
terrytlu force-pushed the AMORO-4292-orphan-clean-location-conflict branch from bd9a4b6 to 9c2d96d Compare August 14, 2026 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ams-server Ams server module module:common type:docs Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: IcebergTableMaintainer orphan file cleanup can corrupt other tables sharing the same location

1 participant