Skip to content

feat(maintenance): RemoveOrphanFilesAction with dry-run-first safety#2755

Open
raghav-reglobe wants to merge 9 commits into
apache:mainfrom
raghav-reglobe:remove-orphan-files
Open

feat(maintenance): RemoveOrphanFilesAction with dry-run-first safety#2755
raghav-reglobe wants to merge 9 commits into
apache:mainfrom
raghav-reglobe:remove-orphan-files

Conversation

@raghav-reglobe

Copy link
Copy Markdown

Summary

Adds maintenance::RemoveOrphanFilesAction: identify — and, unless dry-run, delete — files under the table location that no retained snapshot or table metadata references. This revives the table-maintenance direction proposed in #1453, modeled on iceberg-go's Table.DeleteOrphanFiles (itself modeled on Java's DeleteOrphanFiles action).

Note

Stacked on #2754 (FileIO::list_prefix) — the first commit here is that PR; the net-new to review is the second commit. Happy to rebase once #2754 lands.

Safety design (deleting the wrong file is unrecoverable)

  • Dry-run first: dry_run(true) produces the full orphan report with zero writes, so callers can verify it (e.g. against an independent engine's all_files metadata table) before ever running delete mode.
  • Age guard: only files last-modified before older_than_ms (default now − 3 days, the Java/Go default) are candidates — an in-flight commit's freshly-written files are unreferenced until the commit lands. Files with an unknown modification time are never touched.
  • Conservative referenced set: current + historical metadata files, the version hint, statistics + partition statistics, and every retained snapshot's manifest list, manifests, and all entries' file paths regardless of entry status. Including deleted-status entries means the action can never remove a file any retained manifest still mentions, and keeps the report directly verifiable against Java's all_files metadata table (which also includes logically-deleted files). This is a deliberate, documented divergence from iceberg-go's discardDeleted behavior.
  • PrefixMismatchMode (Error default / Ignore / Delete) with equal_schemes / equal_authorities, matching iceberg-go's handling of files referenced under equivalent URI schemes/authorities.
  • Deletion is bounded-concurrency and best-effort, with per-file failures reported rather than aborting the sweep.

Validation

  • URI-normalization unit tests; end-to-end tests on a real table (memory catalog + parquet writer chain): the dry-run report contains exactly the planted garbage and never a referenced file, the age guard protects fresh files, and delete removes only the garbage — the table remains fully readable and a second pass converges to zero.
  • Full iceberg suite green; cargo fmt clean; public-api.txt regenerated.

raghav-reglobe and others added 2 commits July 2, 2026 08:55
Adds a listing capability to FileIO, needed by maintenance operations
(e.g. orphan-file cleanup) that must enumerate everything under a table
location and compare against paths recorded in table metadata:

- ListEntry { path, size, last_modified_ms } — paths are returned
  absolute, in the same form as the listed prefix, so they round-trip to
  other FileIO operations and compare directly against metadata paths.
  last_modified_ms is optional: age-based logic must treat files with an
  unknown modification time as not eligible.
- Storage::list_prefix with a FeatureUnsupported default, implemented
  for the OpenDAL storages (recursive lister; entries mapped back to
  absolute form), the resolving router, local fs (recursive walk with
  real mtimes), and memory (which now stamps write times so age logic
  is testable).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Raghvendra Singh <raghav@cashify.in>
…s with dry-run-first safety

Adds a maintenance action (not a snapshot commit) that identifies and,
unless dry-run, deletes files under the table location that no retained
snapshot or table metadata references. Modeled on iceberg-go's
Table.DeleteOrphanFiles (itself modeled on Java's DeleteOrphanFiles),
reviving the table-maintenance proposal from apache#1453.

- Dry-run mode returns the full orphan report with zero writes.
- Age guard (default now-3d): files newer than the cutoff are never
  touched — an in-flight commit's freshly-written files are unreferenced
  until the commit lands. Files without a modification time are never
  touched either.
- PrefixMismatchMode (Error default / Ignore / Delete) with
  equal-schemes / equal-authorities URI equivalence, matching
  iceberg-go's handling of files referenced under equivalent schemes.
- The referenced set covers current + historical metadata, the version
  hint, statistics + partition statistics, and every retained snapshot's
  manifest list, manifests, and ALL entries' file paths regardless of
  entry status — an entry-status-agnostic set can never delete a file a
  retained manifest still mentions, and keeps the report verifiable
  against the Java all_files metadata table.
- Deletion is bounded-concurrency and best-effort with per-file failure
  reporting.

Tests: URI-normalization units plus end-to-end on a real table (memory
catalog + parquet writer chain): the dry-run report contains exactly the
planted garbage and never a referenced file, the age guard protects
fresh files, and delete removes only the garbage with the table fully
readable afterwards and a second pass converging to zero.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Raghvendra Singh <raghav@cashify.in>
raghav-reglobe and others added 7 commits July 2, 2026 09:26
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Raghvendra Singh <raghav@cashify.in>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Raghvendra Singh <raghav@cashify.in>
… with tests

Per review: document on FileIO::list_prefix (and the Storage trait) that
the prefix is treated as a directory-style location — implementations
normalize a prefix without a trailing '/' by appending one, so a path
naming an existing file yields an empty list, a trailing-slash prefix is
equivalent to the non-slash form, and a missing prefix is empty rather
than an error. Java's SupportsPrefixOperations.listPrefix leaves the
exact-file/raw-prefix case implementation-defined; this contract fixes it
to directory-style uniformly so callers do not have to defend against
both behaviors.

Add regression tests for MemoryStorage and LocalFsStorage asserting the
exact-file-path case returns empty and the trailing-slash equivalence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Raghvendra Singh <raghav@cashify.in>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant