[GitHub] [accumulo] ctubbsii commented on a change in pull request #1367: Retry new Bulk import on merge. Fixes #471

[GitBox <gi...@apache.org>]

ctubbsii commented on a change in pull request #1367: Retry new Bulk import on merge. Fixes #471
URL: https://github.com/apache/accumulo/pull/1367#discussion_r326418460
 
 

 ##########
 File path: core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java
 ##########
 @@ -325,14 +325,21 @@ private void finishFateOperation(long opid) throws ThriftSecurityException, TExc
 
   public String doBulkFateOperation(List<ByteBuffer> args, String tableName)
       throws AccumuloSecurityException, AccumuloException, TableNotFoundException {
-    try {
-      return doFateOperation(FateOperation.TABLE_BULK_IMPORT2, args, Collections.emptyMap(),
-          tableName);
-    } catch (TableExistsException | NamespaceExistsException e) {
-      // should not happen
-      throw new AssertionError(e);
-    } catch (NamespaceNotFoundException ne) {
-      throw new TableNotFoundException(null, tableName, "Namespace not found", ne);
+    // It is possible a tablet merged before the tablet was locked so retry until we are
+    // either successful or another error happens
+    while (true) {
 
 Review comment:
   This retry loop could be pushed down into the doFateOperation implementation, to get it closer to the RPC. That way, if you add a new enum type to TableOperationExceptionType, you can check it before it is wrapped. This also prevents the thrift types from spreading too far away from the RPC code.
   
   Something like the following in `doFateOperation` would work:
   ```java
       boolean retry = false;
       do {
         try {
           // ... do fate RPC calls here
         } catch (ThriftTableOperationException e) {
           switch (e.getType()) {
             // ... existing code
             case BULK_WHILE_CONCURRENTLY_MERGING_SO_RETRY: // pick a better name for the enum
                // log msg
                retry = true;
                break;
             // ... existing code
           }
         }
       } while(retry);

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services