This is an automated email from the ASF dual-hosted git repository.
etudenhoefner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 681b09ddc9 Core: Move Javadoc about commit retries to SnapshotProducer
(#10995)
681b09ddc9 is described below
commit 681b09ddc95e66b056aa7954c6606c5ff329cb24
Author: gaborkaszab <[email protected]>
AuthorDate: Mon Oct 28 10:13:50 2024 +0100
Core: Move Javadoc about commit retries to SnapshotProducer (#10995)
---
.../main/java/org/apache/iceberg/FastAppend.java | 8 +------
.../main/java/org/apache/iceberg/MergeAppend.java | 8 +------
.../java/org/apache/iceberg/SnapshotProducer.java | 6 ++++++
.../java/org/apache/iceberg/StreamingDelete.java | 8 +------
.../java/org/apache/iceberg/TestFastAppend.java | 25 ++++++++++++++++++++++
5 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/core/src/main/java/org/apache/iceberg/FastAppend.java
b/core/src/main/java/org/apache/iceberg/FastAppend.java
index 1b6e1b3b52..1e2f6fe0d9 100644
--- a/core/src/main/java/org/apache/iceberg/FastAppend.java
+++ b/core/src/main/java/org/apache/iceberg/FastAppend.java
@@ -24,7 +24,6 @@ import java.util.Map;
import java.util.Set;
import org.apache.iceberg.encryption.EncryptedOutputFile;
import org.apache.iceberg.events.CreateSnapshotEvent;
-import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.exceptions.RuntimeIOException;
import org.apache.iceberg.io.InputFile;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
@@ -32,12 +31,7 @@ import
org.apache.iceberg.relocated.com.google.common.collect.Iterables;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.util.DataFileSet;
-/**
- * {@link AppendFiles Append} implementation that adds a new manifest file for
the write.
- *
- * <p>This implementation will attempt to commit 5 times before throwing {@link
- * CommitFailedException}.
- */
+/** {@link AppendFiles Append} implementation that adds a new manifest file
for the write. */
class FastAppend extends SnapshotProducer<AppendFiles> implements AppendFiles {
private final String tableName;
private final TableOperations ops;
diff --git a/core/src/main/java/org/apache/iceberg/MergeAppend.java
b/core/src/main/java/org/apache/iceberg/MergeAppend.java
index 3ef553ba78..231ad8cc5d 100644
--- a/core/src/main/java/org/apache/iceberg/MergeAppend.java
+++ b/core/src/main/java/org/apache/iceberg/MergeAppend.java
@@ -18,15 +18,9 @@
*/
package org.apache.iceberg;
-import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
-/**
- * Append implementation that produces a minimal number of manifest files.
- *
- * <p>This implementation will attempt to commit 5 times before throwing {@link
- * CommitFailedException}.
- */
+/** {@link AppendFiles Append} implementation that produces a minimal number
of manifest files. */
class MergeAppend extends MergingSnapshotProducer<AppendFiles> implements
AppendFiles {
MergeAppend(String tableName, TableOperations ops) {
super(tableName, ops);
diff --git a/core/src/main/java/org/apache/iceberg/SnapshotProducer.java
b/core/src/main/java/org/apache/iceberg/SnapshotProducer.java
index 33114baa64..89f9eab719 100644
--- a/core/src/main/java/org/apache/iceberg/SnapshotProducer.java
+++ b/core/src/main/java/org/apache/iceberg/SnapshotProducer.java
@@ -80,6 +80,12 @@ import org.apache.iceberg.util.ThreadPools;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+/**
+ * Keeps common functionality to create a new snapshot.
+ *
+ * <p>The number of attempted commits is controlled by {@link
TableProperties#COMMIT_NUM_RETRIES}
+ * and {@link TableProperties#COMMIT_NUM_RETRIES_DEFAULT} properties.
+ */
@SuppressWarnings("UnnecessaryAnonymousClass")
abstract class SnapshotProducer<ThisT> implements SnapshotUpdate<ThisT> {
private static final Logger LOG =
LoggerFactory.getLogger(SnapshotProducer.class);
diff --git a/core/src/main/java/org/apache/iceberg/StreamingDelete.java
b/core/src/main/java/org/apache/iceberg/StreamingDelete.java
index df5a11bf31..81621164e4 100644
--- a/core/src/main/java/org/apache/iceberg/StreamingDelete.java
+++ b/core/src/main/java/org/apache/iceberg/StreamingDelete.java
@@ -18,15 +18,9 @@
*/
package org.apache.iceberg;
-import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.expressions.Expression;
-/**
- * {@link DeleteFiles Delete} implementation that avoids loading full
manifests in memory.
- *
- * <p>This implementation will attempt to commit 5 times before throwing {@link
- * CommitFailedException}.
- */
+/** {@link DeleteFiles Delete} implementation that avoids loading full
manifests in memory. */
public class StreamingDelete extends MergingSnapshotProducer<DeleteFiles>
implements DeleteFiles {
private boolean validateFilesToDeleteExist = false;
diff --git a/core/src/test/java/org/apache/iceberg/TestFastAppend.java
b/core/src/test/java/org/apache/iceberg/TestFastAppend.java
index 7a93b99887..8da9cb0e33 100644
--- a/core/src/test/java/org/apache/iceberg/TestFastAppend.java
+++ b/core/src/test/java/org/apache/iceberg/TestFastAppend.java
@@ -271,6 +271,31 @@ public class TestFastAppend extends TestBase {
assertThat(new File(newManifest.path())).doesNotExist();
}
+ @TestTemplate
+ public void testIncreaseNumRetries() {
+ TestTables.TestTableOperations ops = table.ops();
+ ops.failCommits(TableProperties.COMMIT_NUM_RETRIES_DEFAULT + 1);
+
+ AppendFiles append = table.newFastAppend().appendFile(FILE_B);
+
+ // Default number of retries results in a failed commit
+ assertThatThrownBy(append::commit)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessage("Injected failure");
+
+ // After increasing the number of retries the commit succeeds
+ table
+ .updateProperties()
+ .set(
+ TableProperties.COMMIT_NUM_RETRIES,
+ String.valueOf(TableProperties.COMMIT_NUM_RETRIES_DEFAULT + 1))
+ .commit();
+
+ append.commit();
+
+ validateSnapshot(null, readMetadata().currentSnapshot(), FILE_B);
+ }
+
@TestTemplate
public void testAppendManifestCleanup() throws IOException {
// inject 5 failures
