fightBoxing commented on issue #17338: URL: https://github.com/apache/iceberg/issues/17338#issuecomment-5082187358
## Fix Proposal: PR #17375 I have created a PR to fix this race condition: https://github.com/apache/iceberg/pull/17375 ### Root Cause Analysis The problem is in `CachingCatalog.loadTable()` where a non-atomic sequence of `getIfPresent` → `get` → `put` allows a concurrent `invalidate()` to be overwritten by a stale `put`: ``` Thread A: check cache → miss, start loading value Thread B: change external state → invalidate() Thread A: put(stale value) ← stale value resurrected in cache ``` ### Fix Replaced the non-atomic sequence with `tableCache.asMap().compute()`, which uses `ConcurrentHashMap.compute()` under the hood. This guarantees that `compute` and `remove` (invalidate) are serialized on the same key, preventing the stale put from overwriting a prior invalidate. ### Verification - Added regression test `testConcurrentLoadAndInvalidateDoesNotResurrectStaleEntry` that stress-tests 200 iterations of concurrent `loadTable` + `invalidateTable`. - All 13 existing `TestCachingCatalog` tests pass without regression. - `BUILD SUCCESSFUL` on `:iceberg-core:test`. -- 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]
