From 944bbcf8e5874bed8454dfa5fe605e81ef66c753 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Sat, 26 Sep 2026 08:01:19 -0600 Subject: [PATCH] test: backport the upstream fix for a flaky Iceberg 1.9.1 rewrite test TestRewriteDataFilesAction.testParallelPartialProgressWithMaxFailedCommitsLargerThanTotalFileGroup fails now and then in the Iceberg 1.9 shard-2 job with "1 rewrite commits failed. This is more than the maximum allowed failures of 0". Three threads commit ten file groups one at a time against a Hadoop table, and a commit that keeps losing the race runs out of Iceberg's default four commit retries. The test allows no failed commits, so that one starved commit fails it. This is apache/iceberg#12889, and it happens in Iceberg's commit code, not Comet's. Upstream fixed the test in apache/iceberg#13208, which allows one failed commit, and apache/iceberg#13598, which then expects at least 10 snapshots instead of exactly 11. Both are in 1.10.0 but not 1.9.1. Regenerate the 1.9.1 diff with both applied to the spark/v3.4 and spark/v3.5 copies of the test, as upstream applied them to every Spark version. The method now matches 1.10.0's. --- dev/diffs/iceberg/1.9.1.diff | 70 ++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/dev/diffs/iceberg/1.9.1.diff b/dev/diffs/iceberg/1.9.1.diff index 4b757ceaa56..19484bfa380 100644 --- a/dev/diffs/iceberg/1.9.1.diff +++ b/dev/diffs/iceberg/1.9.1.diff @@ -1085,6 +1085,41 @@ index 3e8953fb95..2fd144ff74 100644 .enableHiveSupport() .getOrCreate(); +diff --git a/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java b/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java +index dfe8f51728..d290ac3f1d 100644 +--- a/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java ++++ b/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java +@@ -1321,7 +1321,7 @@ public class TestRewriteDataFilesAction extends TestBase { + } + + @TestTemplate +- public void testParallelPartialProgressWithMaxFailedCommitsLargerThanTotalFileGroup() { ++ public void testParallelPartialProgressWithMaxCommitsLargerThanTotalGroupCount() { + Table table = createTable(20); + int fileSize = averageFileSize(table); + +@@ -1336,14 +1336,19 @@ public class TestRewriteDataFilesAction extends TestBase { + // Since we can have at most one commit per file group and there are only 10 file + // groups, actual number of commits is 10 + .option(RewriteDataFiles.PARTIAL_PROGRESS_MAX_COMMITS, "20") +- .option(RewriteDataFiles.PARTIAL_PROGRESS_MAX_FAILED_COMMITS, "0"); ++ // Setting max-failed-commits to 1 to tolerate random commit failure ++ .option(RewriteDataFiles.PARTIAL_PROGRESS_MAX_FAILED_COMMITS, "1"); + rewrite.execute(); + + table.refresh(); + + List postRewriteData = currentData(); + assertEquals("We shouldn't have changed the data", originalData, postRewriteData); +- shouldHaveSnapshots(table, 11); ++ table.refresh(); ++ assertThat(table.snapshots()) ++ .as("Table did not have the expected number of snapshots") ++ // To tolerate 1 random commit failure ++ .hasSizeGreaterThanOrEqualTo(10); + shouldHaveNoOrphans(table); + shouldHaveACleanCache(table); + } diff --git a/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/source/ScanTestBase.java b/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/source/ScanTestBase.java index 06d5e0c44f..d60cb9467a 100644 --- a/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/source/ScanTestBase.java @@ -2827,6 +2862,41 @@ index 3e9f3334ef..187d2c8435 100644 .enableHiveSupport() .getOrCreate(); +diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java +index dfe8f51728..d290ac3f1d 100644 +--- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java ++++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java +@@ -1321,7 +1321,7 @@ public class TestRewriteDataFilesAction extends TestBase { + } + + @TestTemplate +- public void testParallelPartialProgressWithMaxFailedCommitsLargerThanTotalFileGroup() { ++ public void testParallelPartialProgressWithMaxCommitsLargerThanTotalGroupCount() { + Table table = createTable(20); + int fileSize = averageFileSize(table); + +@@ -1336,14 +1336,19 @@ public class TestRewriteDataFilesAction extends TestBase { + // Since we can have at most one commit per file group and there are only 10 file + // groups, actual number of commits is 10 + .option(RewriteDataFiles.PARTIAL_PROGRESS_MAX_COMMITS, "20") +- .option(RewriteDataFiles.PARTIAL_PROGRESS_MAX_FAILED_COMMITS, "0"); ++ // Setting max-failed-commits to 1 to tolerate random commit failure ++ .option(RewriteDataFiles.PARTIAL_PROGRESS_MAX_FAILED_COMMITS, "1"); + rewrite.execute(); + + table.refresh(); + + List postRewriteData = currentData(); + assertEquals("We shouldn't have changed the data", originalData, postRewriteData); +- shouldHaveSnapshots(table, 11); ++ table.refresh(); ++ assertThat(table.snapshots()) ++ .as("Table did not have the expected number of snapshots") ++ // To tolerate 1 random commit failure ++ .hasSizeGreaterThanOrEqualTo(10); + shouldHaveNoOrphans(table); + shouldHaveACleanCache(table); + } diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/AvroDataTest.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/AvroDataTest.java index a31138ae01..501c8ac93e 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/AvroDataTest.java