[accumulo] branch master updated: Handle spurious failure were rename succeeded but reported it failed (#1588) (#1594)

[kturner]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new cd4c234  Handle spurious failure were rename succeeded but reported it 
failed (#1588) (#1594)
cd4c234 is described below

commit cd4c234f64150fd1e9543bb31e4acefcfd822eb6
Author: Tushar Dhadiwal <tushardhadi...@users.noreply.github.com>
AuthorDate: Tue Apr 28 07:27:55 2020 -0700

    Handle spurious failure were rename succeeded but reported it failed 
(#1588) (#1594)
    
    PR addresses a few issues:
        Spurious rename failures where rename failed but actually succeeded
        The check for file existence should only be done before attempting 
rename
        Should not delete file if it exists (an admin may want to investigate).
    
    Co-authored-by: Tushar D <tudha...@microsoft.com>
---
 .../apache/accumulo/tserver/tablet/DatafileManager.java  | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
index 46fae64..0639610 100644
--- 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
+++ 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
@@ -329,16 +329,26 @@ class DatafileManager {
     StoredTabletFile newFile;
     // rename before putting in metadata table, so files in metadata table 
should
     // always exist
+    boolean attemptedRename = false;
     do {
       try {
         if (dfv.getNumEntries() == 0) {
+
           
tablet.getTabletServer().getFileSystem().deleteRecursively(tmpDatafile.getPath());
         } else {
-          if 
(tablet.getTabletServer().getFileSystem().exists(newDatafile.getPath())) {
+          if (!attemptedRename
+              && 
tablet.getTabletServer().getFileSystem().exists(newDatafile.getPath())) {
             log.warn("Target map file already exist {}", newDatafile);
-            
tablet.getTabletServer().getFileSystem().deleteRecursively(newDatafile.getPath());
+            throw new RuntimeException("File unexpectedly exists " + 
newDatafile.getPath());
           }
-
+          // the following checks for spurious rename failures that succeeded 
but gave an IoE
+          if (attemptedRename
+              && 
tablet.getTabletServer().getFileSystem().exists(newDatafile.getPath())
+              && 
!tablet.getTabletServer().getFileSystem().exists(tmpDatafile.getPath())) {
+            // seems like previous rename succeeded, so break
+            break;
+          }
+          attemptedRename = true;
           rename(tablet.getTabletServer().getFileSystem(), 
tmpDatafile.getPath(),
               newDatafile.getPath());
         }