Repository: hbase
Updated Branches:
  refs/heads/branch-2.0 0d2b18bf2 -> e72a1c694


HBASE-20330 ProcedureExecutor.start() gets stuck in recover lease on store

rollWriter() fails after creating the file and returns false. In next iteration 
of while loop in recoverLease() file list is refreshed.

Signed-off-by: Appy <a...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/e72a1c69
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e72a1c69
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e72a1c69

Branch: refs/heads/branch-2.0
Commit: e72a1c694ef2db4c753fdb0b40ddb8023009dfae
Parents: 0d2b18b
Author: Umesh Agashe <uaga...@cloudera.com>
Authored: Wed Apr 4 16:07:50 2018 -0700
Committer: Michael Stack <st...@apache.org>
Committed: Wed Apr 11 16:06:17 2018 -0700

----------------------------------------------------------------------
 hbase-procedure/pom.xml                         |  5 +++
 .../procedure2/store/wal/WALProcedureStore.java | 13 ++++---
 .../store/wal/TestWALProcedureStore.java        | 37 ++++++++++++++++++++
 3 files changed, 50 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/e72a1c69/hbase-procedure/pom.xml
----------------------------------------------------------------------
diff --git a/hbase-procedure/pom.xml b/hbase-procedure/pom.xml
index f9ad9f3..562d148 100644
--- a/hbase-procedure/pom.xml
+++ b/hbase-procedure/pom.xml
@@ -100,6 +100,11 @@
       <groupId>org.apache.hbase</groupId>
       <artifactId>hbase-metrics-api</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <profiles>

http://git-wip-us.apache.org/repos/asf/hbase/blob/e72a1c69/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java
----------------------------------------------------------------------
diff --git 
a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java
 
b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java
index b718fe7..d14e1bf 100644
--- 
a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java
+++ 
b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java
@@ -359,21 +359,20 @@ public class WALProcedureStore extends ProcedureStoreBase 
{
     lock.lock();
     try {
       LOG.trace("Starting WAL Procedure Store lease recovery");
-      FileStatus[] oldLogs = getLogFiles();
       while (isRunning()) {
+        FileStatus[] oldLogs = getLogFiles();
         // Get Log-MaxID and recover lease on old logs
         try {
           flushLogId = initOldLogs(oldLogs);
         } catch (FileNotFoundException e) {
           LOG.warn("Someone else is active and deleted logs. retrying.", e);
-          oldLogs = getLogFiles();
           continue;
         }
 
         // Create new state-log
         if (!rollWriter(flushLogId + 1)) {
           // someone else has already created this log
-          LOG.debug("Someone else has already created log " + flushLogId);
+          LOG.debug("Someone else has already created log {}. Retrying.", 
flushLogId);
           continue;
         }
 
@@ -1002,7 +1001,8 @@ public class WALProcedureStore extends ProcedureStoreBase 
{
     return true;
   }
 
-  private boolean rollWriter(final long logId) throws IOException {
+  @VisibleForTesting
+  boolean rollWriter(final long logId) throws IOException {
     assert logId > flushLogId : "logId=" + logId + " flushLogId=" + flushLogId;
     assert lock.isHeldByCurrentThread() : "expected to be the lock owner. " + 
lock.isLocked();
 
@@ -1072,7 +1072,10 @@ public class WALProcedureStore extends 
ProcedureStoreBase {
   }
 
   private void closeCurrentLogStream() {
-    if (stream == null) return;
+    if (stream == null || logs.isEmpty()) {
+      return;
+    }
+
     try {
       ProcedureWALFile log = logs.getLast();
       log.setProcIds(storeTracker.getUpdatedMinProcId(), 
storeTracker.getUpdatedMaxProcId());

http://git-wip-us.apache.org/repos/asf/hbase/blob/e72a1c69/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/TestWALProcedureStore.java
----------------------------------------------------------------------
diff --git 
a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/TestWALProcedureStore.java
 
b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/TestWALProcedureStore.java
index 1929c0c..64cf211 100644
--- 
a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/TestWALProcedureStore.java
+++ 
b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/TestWALProcedureStore.java
@@ -54,6 +54,9 @@ import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -760,6 +763,40 @@ public class TestWALProcedureStore {
   }
 
   @Test
+  public void testLogFileAleadExists() throws IOException {
+    final boolean[] tested = {false};
+    WALProcedureStore mStore = Mockito.spy(procStore);
+
+    Answer<Boolean> ans = new Answer<Boolean>() {
+      @Override
+      public Boolean answer(InvocationOnMock invocationOnMock) throws 
Throwable {
+        long logId = ((Long) invocationOnMock.getArgument(0)).longValue();
+        switch ((int) logId) {
+          case 2:
+            // Create a file so that real rollWriter() runs into file exists 
condition
+            Path logFilePath = mStore.getLogFilePath(logId);
+            mStore.getFileSystem().create(logFilePath);
+            break;
+          case 3:
+            // Success only when we retry with logId 3
+            tested[0] = true;
+          default:
+            break;
+        }
+        return (Boolean) invocationOnMock.callRealMethod();
+      }
+    };
+
+    // First time Store has one log file, next id will be 2
+    Mockito.doAnswer(ans).when(mStore).rollWriter(2);
+    // next time its 3
+    Mockito.doAnswer(ans).when(mStore).rollWriter(3);
+
+    mStore.recoverLease();
+    assertTrue(tested[0]);
+  }
+
+  @Test
   public void testLoadChildren() throws Exception {
     TestProcedure a = new TestProcedure(1, 0);
     TestProcedure b = new TestProcedure(2, 1);

Reply via email to