amogh-jahagirdar commented on code in PR #11131:
URL: https://github.com/apache/iceberg/pull/11131#discussion_r1805071134
##########
core/src/main/java/org/apache/iceberg/ManifestFilterManager.java:
##########
@@ -111,6 +115,10 @@ protected void failMissingDeletePaths() {
this.failMissingDeletePaths = true;
}
+ protected void useReferencedManifests(boolean
useReferencedManifestsForFiltering) {
Review Comment:
So I have a few tests now, one is testing a concurrent manifest rewrite +
row delta which verifies that the row delta filter manager state gets updated
(not directly since this is all package private) but via verifying the output
manifests. Another test I added is the same concurrent manifest rewrite + row
delta, but the row delta is performed in a transaction. The row delta operation
internal to the transaction is committed, and then the concurrent manifest
rewrite is committed, and then the overall row delta transaction is committed.
This flow forces the row delta transaction to retry and switch from using
referenced manifests as a source of truth to not using it as a source of truth
since the subset check fails due to the new manifests not containing all the
referenced manifests. Same test for overwrite as well.
If the logic on trusting referenced manifests changes to returning true when
they shouldn't, these tests will fail since we end up skipping rewriting the
manifest we should end up re-writing. So I think these tests cover what we need
to cover.
--
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]
