From aa9bee75de4df081c2702ca6fe057cb3633ec358 Mon Sep 17 00:00:00 2001 From: Jason Fine Date: Wed, 6 Dec 2023 22:43:24 +0200 Subject: [PATCH] Changed test in FastAppend to testy retry with transaction --- .../java/org/apache/iceberg/FastAppend.java | 2 +- .../org/apache/iceberg/TestFastAppend.java | 26 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/FastAppend.java b/core/src/main/java/org/apache/iceberg/FastAppend.java index 21d4791a0ab1..50cad294c570 100644 --- a/core/src/main/java/org/apache/iceberg/FastAppend.java +++ b/core/src/main/java/org/apache/iceberg/FastAppend.java @@ -187,7 +187,7 @@ protected void cleanUncommitted(Set committed) { deleteFile(manifest.path()); } } - this.newManifests = null; + this.newManifests = committedNewManifests; } // clean up only rewrittenAppendManifests as they are always owned by the table diff --git a/core/src/test/java/org/apache/iceberg/TestFastAppend.java b/core/src/test/java/org/apache/iceberg/TestFastAppend.java index d8155f0b7a80..d39e0e4a1aad 100644 --- a/core/src/test/java/org/apache/iceberg/TestFastAppend.java +++ b/core/src/test/java/org/apache/iceberg/TestFastAppend.java @@ -314,34 +314,34 @@ public void testRecoveryWithoutManifestList() { } @Test - public void testRecoveryWithManualReCommit() { + public void testRecoveryWithTransaction() { TestTables.TestTableOperations ops = table.ops(); ops.failCommits(5); - AppendFiles append = table.newFastAppend().appendFile(FILE_B); + Transaction transaction = table.newTransaction(); + AppendFiles append = transaction.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()); + ManifestFile originalManifest = pending.allManifests(FILE_IO).get(0); + append.commit(); - Assertions.assertThatThrownBy(append::commit) + Assertions.assertThatThrownBy(transaction::commitTransaction) .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(); + transaction.commitTransaction(); 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)); + "Original manifest from before retry should exist", + new File(originalManifest.path()).exists()); + Assert.assertTrue( + "Should commit the original manifest created before retrying the transaction", + metadata.currentSnapshot().allManifests(FILE_IO).contains(originalManifest)); } @Test