yassbibi opened a new pull request, #4941:
URL: https://github.com/apache/polaris/pull/4941
Iceberg v2 delete manifests caused the cleanup task to fail silently because
ManifestFiles.read() rejects them with IllegalArgumentException. Route to
ManifestFiles.readDeleteManifest() when the manifest's content type is DELETES,
so DROP TABLE PURGE no longer leaves position-delete files and their manifests
as orphans in object storage.
## Summary
Fix #4891: `ManifestFileCleanupTaskHandler` crashed silently on Iceberg v2
delete
manifests, leaving position-delete files as orphans in object storage after
`DROP TABLE PURGE`.
`cleanUpManifestFile()` unconditionally called `ManifestFiles.read()`, which
rejects delete manifests (`content() == DELETES`) with
`IllegalArgumentException`.
The exception is a `RuntimeException` and was not caught, so the async
cleanup
task failed silently while the DROP returned success to the client.
## Changes
- Extract `openManifestReader()` helper that switches on
`manifestFile.content()`
to route DATA manifests to `ManifestFiles.read()` and DELETES manifests to
`ManifestFiles.readDeleteManifest()`.
- Use `ManifestReader<? extends ContentFile<?>>` so iteration works for both
`DataFile` and `DeleteFile`.
- Update log messages to refer to "content files" instead of "data files".
- Add `TaskTestUtils.deleteManifestFile()` helper to write a v2 delete
manifest
with position-delete file entries.
- Add `testCleanupDeleteManifest` covering the new code path.
- Update CHANGELOG `[Unreleased] / Fixes`.
## Validation
- New unit test (`testCleanupDeleteManifest`) reproduces the bug scenario and
passes with the fix. All existing tests in
`ManifestFileCleanupTaskHandlerTest`
still pass (no regression for data manifests).
- End-to-end verified on a downstream deployment (Trino + Polaris + S3):
v2 table โ `DELETE โฆ WHERE` โ `DROP TABLE โฆ PURGE` no longer leaves
orphan files in the bucket.
## Checklist
- [x] ๐ก๏ธ Don't disclose security issues
- [x] ๐ Clearly explained why the changes are needed: Fixes #4891
- [x] ๐งช Added unit test covering the new code path; e2e tested downstream
- [x] ๐ก Comments added where helpful (helper rationale)
- [x] ๐งพ Updated CHANGELOG.md
- [ ] ๐ site/content/in-dev/unreleased โ N/A (restores documented behavior,
no config/API change)
## AI assistance
Drafted with assistance from Claude Code; design, validation, and review
performed by me. End-to-end testing executed on a downstream deployment
under my control.
Fixes #4891
--
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]
