lilei1128 commented on code in PR #16980:
URL: https://github.com/apache/iceberg/pull/16980#discussion_r3487484137
##########
core/src/test/java/org/apache/iceberg/TestDeleteFiles.java:
##########
@@ -738,6 +740,32 @@ public void
removingDataFilesWhenTruncatingAlsoRemovesDVs() {
statuses(Status.DELETED, Status.DELETED));
}
+ @TestTemplate
+ public void testConcurrentManifestFilteringWithExpression() {
Review Comment:
Thanks for sharing #16984 and the collidingLocation technique — forcing all
paths into the same hash bucket is a much more reliable way to reproduce the
race than simply increasing file count, and we will adapt that approach in our
test.
One note on the synchronizedSet approach: as your own comment acknowledges,
synchronizedSet only protects individual add/contains calls — iteration is
not covered. validateRequiredDeletes uses containsAll and buildSummary iterates
over deleteFiles, both of which are safe today only because they run after
filtering completes. That's a correctness constraint buried in call-site
ordering rather than enforced by the data structure itself.
Our approach (collect into a ConcurrentLinkedQueue during parallel
filtering, then merge into deleteFiles on the calling thread after Tasks.range
returns) avoids this entirely — deleteFiles is never written concurrently, so
iteration safety is guaranteed structurally rather than by convention.
--
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]
