rdblue commented on code in PR #9230:
URL: https://github.com/apache/iceberg/pull/9230#discussion_r1417819364


##########
core/src/test/java/org/apache/iceberg/TestFastAppend.java:
##########
@@ -313,6 +313,37 @@ public void testRecoveryWithoutManifestList() {
         
metadata.currentSnapshot().allManifests(FILE_IO).contains(newManifest));
   }
 
+  @Test
+  public void testRecoveryWithManualReCommit() {
+    TestTables.TestTableOperations ops = table.ops();
+    ops.failCommits(5);
+
+    AppendFiles append = table.newFastAppend().appendFile(FILE_B);
+    Snapshot pending = append.apply();
+    ManifestFile newManifest = pending.allManifests(FILE_IO).get(0);
+    Assert.assertTrue("Should create new manifest", new 
File(newManifest.path()).exists());
+
+    Assertions.assertThatThrownBy(append::commit)
+            .isInstanceOf(CommitFailedException.class)
+            .hasMessage("Injected failure");
+
+    TableMetadata metadata = readMetadata();
+
+    Assert.assertNull("No snapshot is committed", metadata.currentSnapshot());
+
+    pending = append.apply();
+    newManifest = pending.allManifests(FILE_IO).get(0);
+
+    append.commit();
+
+    metadata = readMetadata();
+    validateSnapshot(null, metadata.currentSnapshot(), FILE_B);
+    Assert.assertTrue("Should commit same new manifest", new 
File(newManifest.path()).exists());
+    Assert.assertTrue(
+            "Should commit the same new manifest",
+            
metadata.currentSnapshot().allManifests(FILE_IO).contains(newManifest));

Review Comment:
   This isn't actually testing that the manifest is reused across commit 
attempts because it came from `apply` that was called after the latest failure 
(without a cleanup operation).
   
   Instead of manually exercising this path, why not use a transaction here and 
fail just once to validate?



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to