Core: Support serializable and snapshot isolation for ReplacePartitions #2925

szehon-ho · 2021-08-03T06:16:02Z

From https://mail-archives.apache.org/mod_mbox/iceberg-dev/202107.mbox/%3CCAMwmD1_c4u%2BwDSOLtScCyid%2BfHicBnKdmyTuKqFTG-uaE9eq6A%40mail.gmail.com%3E and offline discussion with Ryan
This change will make "insert overwrite" with mode=dynamic support serializable isolation (a ValidationException will be thrown if a conflicting change is made) in the case where it is not selecting from the same table.
This will catch cases where two insert overwrite happen at the same time, both can potentially succeed and both datas land.
"insert overwrite" remains snapshot isolation in the case where the select is from the same table. For that to support serializable, we need to plug the snapshotId from initial Spark compile, which could be looked at on spark-extension side.

szehon-ho · 2021-08-03T16:38:59Z

@rdblue if you could look when you have time, thanks

core/src/main/java/org/apache/iceberg/BaseReplacePartitions.java

szehon-ho · 2021-08-11T05:35:10Z

Updated, and also turned off serialzable isolation by default (can turn it on as well)

api/src/main/java/org/apache/iceberg/ReplacePartitions.java

core/src/main/java/org/apache/iceberg/BaseReplacePartitions.java

core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java

core/src/test/java/org/apache/iceberg/TestReplacePartitions.java

szehon-ho

@rdblue thanks a lot for taking a look. I rewrote it based on your suggestion (from using Expression to using a filter of partitionSet::contains). Also added a toString method to PartitionSet to make the user-facing message.

core/src/main/java/org/apache/iceberg/BaseReplacePartitions.java

api/src/main/java/org/apache/iceberg/ReplacePartitions.java

core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java

spark3/src/main/java/org/apache/iceberg/spark/source/SparkWrite.java

szehon-ho · 2021-09-22T19:11:20Z

@rdblue thanks for the suggestions. I put validate-from-snapshot as a SparkWriteOption, and added a Spark level test for it based on it, if you want to take another look.

…pression

…ot and add unit test

-Refactor common logic to methods in MergingSnapshotProducer::validate methods -Change serialization level from table properties to spark-write-conf -Other small fixes

szehon-ho · 2022-03-03T03:58:14Z

@aokolnychyi thanks for the review, addressed the comments and implemented snapshot isolation + tests based on the general agreement. Will also change the name of the pr to reflect

aokolnychyi · 2022-03-03T05:44:08Z

Thanks, @szehon-ho! This is awesome work! Thanks others for reviewing too.

This commit bumps the CDP Build number to 27992803 and versions to 7.2.16.0-77. The new version includes apache/iceberg#2925, an Iceberg library improvement to support serializable and snapshot isolation for ReplacePartitions. This is a prereuquisite to safely execute INSERT OVERWRITE entire partitions. Testing: - Ran a build and verified that the downloads succeed. Change-Id: I9fcef254a5f845540e09e0d8d2111f305fb2fea2 Reviewed-on: http://gerrit.cloudera.org:8080/18613 Reviewed-by: Tamas Mate <[email protected]> Tested-by: Tamas Mate <[email protected]>

…ns (apache#2925) (cherry picked from commit 01c62cb)

github-actions bot added API core spark labels Aug 3, 2021

szehon-ho changed the title ~~Support serializable isolation for ReplacePartitions~~ Core: Support serializable isolation for ReplacePartitions Aug 3, 2021

rdblue reviewed Aug 9, 2021

View reviewed changes

core/src/main/java/org/apache/iceberg/BaseReplacePartitions.java Outdated Show resolved Hide resolved

szehon-ho force-pushed the serializable_replace branch from 57af2be to 2406ab9 Compare August 11, 2021 05:32