[iceberg] branch master updated: Let NessieCatalog handle HttpClientException and clean up retry-handling (#2519)

[rymurr]

This is an automated email from the ASF dual-hosted git repository.

rymurr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git


The following commit(s) were added to refs/heads/master by this push:
     new ff28bec  Let NessieCatalog handle HttpClientException and clean up 
retry-handling (#2519)
ff28bec is described below

commit ff28becbc1af5ca9db9712b5bf3e5683d122d9db
Author: Robert Stupp <sn...@snazy.de>
AuthorDate: Mon May 24 16:48:37 2021 +0200

    Let NessieCatalog handle HttpClientException and clean up retry-handling 
(#2519)
---
 .../org/apache/iceberg/nessie/NessieCatalog.java   | 32 +++++++++++-----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java 
b/nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java
index a248cd1..0c84fa0 100644
--- a/nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java
+++ b/nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java
@@ -144,14 +144,23 @@ public class NessieCatalog extends BaseMetastoreCatalog 
implements AutoCloseable
       return false;
     }
 
+    Operations contents = ImmutableOperations.builder()
+        
.addOperations(ImmutableDelete.builder().key(NessieUtil.toKey(identifier)).build())
+        .commitMeta(CommitMeta.fromMessage(String.format("delete table %s", 
identifier)))
+        .build();
+
     // We try to drop the table. Simple retry after ref update.
     boolean threw = true;
     try {
-      Tasks.foreach(identifier)
+      Tasks.foreach(contents)
            .retry(5)
            .stopRetryOn(NessieNotFoundException.class)
            .throwFailureWhenFinished()
-           .run(this::dropTableInner, BaseNessieClientServerException.class);
+           .onFailure((c, exception) -> refresh())
+           .run(c -> {
+             
client.getTreeApi().commitMultipleOperations(reference.getAsBranch().getName(), 
reference.getHash(), c);
+             refresh(); // note: updated to reference.updateReference() with 
Nessie 0.6
+           }, BaseNessieClientServerException.class);
       threw = false;
     } catch (NessieConflictException e) {
       logger.error("Cannot drop table: failed after retry (update ref and 
retry)", e);
@@ -189,6 +198,7 @@ public class NessieCatalog extends BaseMetastoreCatalog 
implements AutoCloseable
           .retry(5)
           .stopRetryOn(NessieNotFoundException.class)
           .throwFailureWhenFinished()
+          .onFailure((c, exception) -> refresh())
           .run(c -> {
             
client.getTreeApi().commitMultipleOperations(reference.getAsBranch().getName(), 
reference.getHash(), c);
             refresh();
@@ -203,6 +213,10 @@ public class NessieCatalog extends BaseMetastoreCatalog 
implements AutoCloseable
     } catch (BaseNessieClientServerException e) {
       throw new CommitFailedException(e, "Failed to rename table: the current 
reference is not up to date.");
     }
+    // Intentionally just "throw through" Nessie's HttpClientException here 
and do not "special case"
+    // just the "timeout" variant to propagate all kinds of network errors 
(e.g. connection reset).
+    // Network code implementation details and all kinds of network devices 
can induce unexpected
+    // behavior. So better be safe than sorry.
   }
 
   /**
@@ -310,20 +324,6 @@ public class NessieCatalog extends BaseMetastoreCatalog 
implements AutoCloseable
     }
   }
 
-
-  public void dropTableInner(TableIdentifier identifier) throws 
NessieConflictException, NessieNotFoundException {
-    try {
-      client.getContentsApi().deleteContents(NessieUtil.toKey(identifier),
-          reference.getAsBranch().getName(),
-          reference.getHash(),
-          String.format("delete table %s", identifier));
-
-    } finally {
-      // TODO: fix this so we don't depend on it in tests. and move refresh 
into catch clause.
-      refresh();
-    }
-  }
-
   private Stream<TableIdentifier> tableStream(Namespace namespace) {
     try {
       return client.getTreeApi()