Skip to content

Core: concurrent manifest filtering corrupts deleteFiles on parallel overwrite#16981

Open
goingforstudying-ctrl wants to merge 5 commits into
apache:mainfrom
goingforstudying-ctrl:fix/manifest-filter-thread-safety
Open

Core: concurrent manifest filtering corrupts deleteFiles on parallel overwrite#16981
goingforstudying-ctrl wants to merge 5 commits into
apache:mainfrom
goingforstudying-ctrl:fix/manifest-filter-thread-safety

Conversation

@goingforstudying-ctrl

Copy link
Copy Markdown

Was debugging a ClassCastException that kept popping up during overwrite operations on tables with many manifests. Took a while to trace it back to the parallel manifest filtering path.

The issue is that filterManifests() uses Tasks.range() to process manifests concurrently across worker threads. Inside filterManifestWithDeletedFiles(), worker threads were directly mutating the shared deleteFiles LinkedHashSet via deleteFiles.add(fileCopy). LinkedHashSet is not thread-safe, so concurrent adds from multiple threads corrupt the internal HashMap, producing the ClassCastException: LinkedHashMap$Entry cannot be cast to HashMap$TreeNode that was reported in #16978.

The fix has three parts:

  1. Collect per-manifest deleted files into a ConcurrentMap from each worker thread instead of mutating the shared set. After all parallel filtering completes, merge the per-manifest results into deleteFiles on the main thread.

  2. Replace the filteredManifestToDeletedFiles HashMap with a ConcurrentHashMap so worker threads can safely put their results without resize races.

  3. Add ThreadSafeFileSet, a ConcurrentHashMap-backed Set implementation for defense-in-depth. Use it for the deleteFiles collection so even if future code accidentally adds concurrent mutations, the set won't corrupt.

I also switched filteredManifestToDeletedFiles to ConcurrentHashMap because worker threads write to it from filterManifest() while the main thread reads after join. The old plain HashMap could resize unpredictably under concurrent writes.

Added TestManifestFilterManagerConcurrency with two tests that create tables with 32 manifests and perform full overwrites/deletes to exercise the parallel filtering path.

Not 100% sure if ThreadSafeFileSet should live in api or core — went with api because it's a general utility, but happy to move it if that doesn't feel right.

Closes #16978

@goingforstudying-ctrl goingforstudying-ctrl force-pushed the fix/manifest-filter-thread-safety branch 5 times, most recently from 57d5b90 to a2553b9 Compare June 28, 2026 08:21
Was debugging a ClassCastException that kept popping up during overwrite
operations on tables with many manifests. Took a while to trace it back
to the parallel manifest filtering path.

The issue is that filterManifests() uses Tasks.range() to process manifests
concurrently across worker threads. Inside filterManifestWithDeletedFiles(),
worker threads were directly mutating the shared deleteFiles LinkedHashSet
via deleteFiles.add(fileCopy). LinkedHashSet is not thread-safe, so
concurrent adds from multiple threads corrupt the internal HashMap,
producing the ClassCastException: LinkedHashMap cannot be cast to
HashMap that was reported in apache#16978.

The fix has three parts:

1. Collect per-manifest deleted files into a ConcurrentMap from each worker
   thread instead of mutating the shared set. After all parallel filtering
   completes, merge the per-manifest results into deleteFiles on the main
   thread.

2. Replace the filteredManifestToDeletedFiles HashMap with a ConcurrentHashMap
   so worker threads can safely put their results without resize races.

3. Add ThreadSafeFileSet, a ConcurrentHashMap-backed Set implementation for
   defense-in-depth. Use it for the deleteFiles collection so even if future
   code accidentally adds concurrent mutations, the set won't corrupt.

I also switched filteredManifestToDeletedFiles to ConcurrentHashMap because
worker threads write to it from filterManifest() while the main thread
reads after join. The old plain HashMap could resize unpredictably under
concurrent writes.

Added TestManifestFilterManagerConcurrency with two tests that create
tables with 32 manifests and perform full overwrites/deletes to exercise
the parallel filtering path.

Closes apache#16978
- Add @parameter annotations and parameters() method for parameterized tests
- Replace @test with @testtemplate for parameterized test extension
- Replace non-existent deleteByRowFilter with deleteFromRowFilter
- Replace non-existent deletedDataFiles with removedDataFiles
- Add missing import for java.util.stream.Stream

These changes fix compilation errors against the current Iceberg API.
@goingforstudying-ctrl goingforstudying-ctrl force-pushed the fix/manifest-filter-thread-safety branch from 17ae08e to 3a71d1a Compare June 28, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Core: ManifestFilterManager thread safety issue during parallel overwrite commit

1 participant