[accumulo] branch 1.9 updated: Fix RecoveryLogReader error on failed file. Closes #961 (#1048)

[mm...@apache.org]

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

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


The following commit(s) were added to refs/heads/1.9 by this push:
     new 995a952  Fix RecoveryLogReader error on failed file. Closes #961 (#1048)
995a952 is described below

commit 995a952b24c11887f5864dd101eae3847b2128f5
Author: Mike Miller <mm...@apache.org>
AuthorDate: Mon Mar 25 15:22:19 2019 -0400

    Fix RecoveryLogReader error on failed file. Closes #961 (#1048)
    
    * Made RecoveryLogReader ignore a failed file so it can recovery properly
    * Added test to RecoveryLogReaderTest
---
 .../accumulo/tserver/log/RecoveryLogReader.java    |  3 +++
 .../tserver/log/RecoveryLogsReaderTest.java        | 25 ++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogReader.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogReader.java
index 718fa98..b892c51 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogReader.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogReader.java
@@ -127,6 +127,9 @@ public class RecoveryLogReader implements CloseableIterator<Entry<LogFileKey,Log
         foundFinish = true;
         continue;
       }
+      if (SortedLogState.FAILED.getMarker().equals(child.getPath().getName())) {
+        continue;
+      }
       FileSystem ns = fs.getVolumeByPath(child.getPath()).getFileSystem();
       heap.add(new Index(new Reader(ns.makeQualified(child.getPath()), ns.getConf())));
     }
diff --git a/server/tserver/src/test/java/org/apache/accumulo/tserver/log/RecoveryLogsReaderTest.java b/server/tserver/src/test/java/org/apache/accumulo/tserver/log/RecoveryLogsReaderTest.java
index 37feafd..2bf806f 100644
--- a/server/tserver/src/test/java/org/apache/accumulo/tserver/log/RecoveryLogsReaderTest.java
+++ b/server/tserver/src/test/java/org/apache/accumulo/tserver/log/RecoveryLogsReaderTest.java
@@ -29,6 +29,7 @@ import java.util.Map.Entry;
 
 import org.apache.accumulo.server.fs.VolumeManager;
 import org.apache.accumulo.server.fs.VolumeManagerImpl;
+import org.apache.accumulo.server.log.SortedLogState;
 import org.apache.accumulo.tserver.log.RecoveryLogReader.SortCheckIterator;
 import org.apache.accumulo.tserver.logger.LogEvents;
 import org.apache.accumulo.tserver.logger.LogFileKey;
@@ -177,4 +178,28 @@ public class RecoveryLogsReaderTest {
     }
   }
 
+  /**
+   * Test a failed marker doesn't cause issues. See Github issue
+   * https://github.com/apache/accumulo/issues/961
+   */
+  @Test
+  public void testFailed() throws Exception {
+    Path manyMaps = new Path("file://" + root.getRoot().getAbsolutePath() + "/manyMaps");
+    fs.create(new Path(manyMaps, SortedLogState.FAILED.getMarker())).close();
+
+    RecoveryLogReader reader = new RecoveryLogReader(fs, manyMaps);
+    IntWritable key = new IntWritable();
+    BytesWritable value = new BytesWritable();
+
+    for (int i = 0; i < 1000; i++) {
+      if (i == 10)
+        continue;
+      assertTrue(reader.next(key, value));
+      assertEquals(i, key.get());
+    }
+    reader.close();
+
+    assertTrue(fs.delete(new Path(manyMaps, SortedLogState.FAILED.getMarker())));
+  }
+
 }