Repository: incubator-ratis
Updated Branches:
  refs/heads/master cb7e6d429 -> 326acb087


RATIS-203. RaftClient request may wait indefinitely for a reply.  Contributed 
by Lokesh Jain


Project: http://git-wip-us.apache.org/repos/asf/incubator-ratis/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ratis/commit/326acb08
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ratis/tree/326acb08
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ratis/diff/326acb08

Branch: refs/heads/master
Commit: 326acb087949498be31ce00e0c0bb0fecc32c1a7
Parents: cb7e6d4
Author: Tsz-Wo Nicholas Sze <[email protected]>
Authored: Mon Feb 5 13:43:42 2018 -0800
Committer: Tsz-Wo Nicholas Sze <[email protected]>
Committed: Mon Feb 5 13:43:42 2018 -0800

----------------------------------------------------------------------
 .../ratis/server/impl/RaftServerImpl.java       | 14 +++++-
 .../ratis/server/storage/SegmentedRaftLog.java  | 14 ++++++
 .../ratis/server/impl/RetryCacheTestUtil.java   | 48 ++++++++++++++++++++
 .../server/storage/TestSegmentedRaftLog.java    | 32 +++++++++++--
 4 files changed, 103 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/326acb08/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java 
b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
index 6f8f603..423837b 100644
--- 
a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
+++ 
b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
@@ -144,7 +144,7 @@ public class RaftServerImpl implements RaftServerProtocol, 
RaftServerAsynchronou
   }
 
   @VisibleForTesting
-  RetryCache getRetryCache() {
+  public RetryCache getRetryCache() {
     return retryCache;
   }
 
@@ -1005,6 +1005,18 @@ public class RaftServerImpl implements 
RaftServerProtocol, RaftServerAsynchronou
     return null;
   }
 
+  public void failClientRequest(LogEntryProto logEntry) {
+    if (logEntry.getLogEntryBodyCase() == 
LogEntryProto.LogEntryBodyCase.SMLOGENTRY) {
+      final ClientId clientId = ClientId.valueOf(logEntry.getClientId());
+      final RetryCache.CacheEntry cacheEntry = getRetryCache().get(clientId, 
logEntry.getCallId());
+      if (cacheEntry != null) {
+        final RaftClientReply reply = new RaftClientReply(clientId, getId(), 
getGroupId(),
+            logEntry.getCallId(), false, null, generateNotLeaderException());
+        cacheEntry.failWithReply(reply);
+      }
+    }
+  }
+
   private class RaftServerJmxAdapter extends JmxRegister implements 
RaftServerMXBean {
     @Override
     public String getId() {

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/326acb08/ratis-server/src/main/java/org/apache/ratis/server/storage/SegmentedRaftLog.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/main/java/org/apache/ratis/server/storage/SegmentedRaftLog.java
 
b/ratis-server/src/main/java/org/apache/ratis/server/storage/SegmentedRaftLog.java
index 76b2bd7..036f3e0 100644
--- 
a/ratis-server/src/main/java/org/apache/ratis/server/storage/SegmentedRaftLog.java
+++ 
b/ratis-server/src/main/java/org/apache/ratis/server/storage/SegmentedRaftLog.java
@@ -318,6 +318,20 @@ public class SegmentedRaftLog extends RaftLog {
                 ServerProtoUtils.toTermIndex(entries[index]), storedEntry,
                 ServerProtoUtils.toString(entries));
           }
+          while (true) {
+            try {
+              final LogEntryProto entry = get(storedEntry.getIndex());
+              server.failClientRequest(entry);
+            } catch (RaftLogIOException e) {
+              LOG.error("Failed to read log " + storedEntry, e);
+            }
+
+            if (iter.hasNext()) {
+              storedEntry = iter.next();
+            } else {
+              break;
+            }
+          }
           break;
         }
       }

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/326acb08/ratis-server/src/test/java/org/apache/ratis/server/impl/RetryCacheTestUtil.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/test/java/org/apache/ratis/server/impl/RetryCacheTestUtil.java
 
b/ratis-server/src/test/java/org/apache/ratis/server/impl/RetryCacheTestUtil.java
new file mode 100644
index 0000000..0d28e2c
--- /dev/null
+++ 
b/ratis-server/src/test/java/org/apache/ratis/server/impl/RetryCacheTestUtil.java
@@ -0,0 +1,48 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ratis.server.impl;
+
+import org.apache.ratis.protocol.ClientId;
+import org.apache.ratis.shaded.proto.RaftProtos;
+import org.apache.ratis.util.TimeDuration;
+import org.junit.Assert;
+
+import java.util.concurrent.TimeUnit;
+
+public class RetryCacheTestUtil {
+  public static RetryCache createRetryCache(){
+    return new RetryCache(5000, TimeDuration.valueOf(60, TimeUnit.SECONDS));
+  }
+
+  public static void createEntry(RetryCache cache, RaftProtos.LogEntryProto 
logEntry){
+    if(logEntry.getLogEntryBodyCase() == 
RaftProtos.LogEntryProto.LogEntryBodyCase.SMLOGENTRY){
+      ClientId clientId = ClientId.valueOf(logEntry.getClientId());
+      long callId = logEntry.getCallId();
+      cache.getOrCreateEntry(clientId, callId);
+    }
+  }
+
+  public static void assertFailure(RetryCache cache,
+      RaftProtos.LogEntryProto logEntry, boolean isFailed) {
+    if(logEntry.getLogEntryBodyCase() == 
RaftProtos.LogEntryProto.LogEntryBodyCase.SMLOGENTRY){
+      ClientId clientId = ClientId.valueOf(logEntry.getClientId());
+      long callId = logEntry.getCallId();
+      Assert.assertEquals(isFailed, cache.get(clientId, callId).isFailed());
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/326acb08/ratis-server/src/test/java/org/apache/ratis/server/storage/TestSegmentedRaftLog.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/test/java/org/apache/ratis/server/storage/TestSegmentedRaftLog.java
 
b/ratis-server/src/test/java/org/apache/ratis/server/storage/TestSegmentedRaftLog.java
index 2a0ec60..a95627f 100644
--- 
a/ratis-server/src/test/java/org/apache/ratis/server/storage/TestSegmentedRaftLog.java
+++ 
b/ratis-server/src/test/java/org/apache/ratis/server/storage/TestSegmentedRaftLog.java
@@ -25,6 +25,9 @@ import org.apache.ratis.protocol.ClientId;
 import org.apache.ratis.protocol.RaftPeerId;
 import org.apache.ratis.server.RaftServerConfigKeys;
 import org.apache.ratis.server.impl.RaftServerConstants;
+import org.apache.ratis.server.impl.RetryCacheTestUtil;
+import org.apache.ratis.server.impl.RetryCache;
+import org.apache.ratis.server.impl.RaftServerImpl;
 import org.apache.ratis.server.protocol.TermIndex;
 import org.apache.ratis.shaded.proto.RaftProtos.LogEntryProto;
 import org.apache.ratis.util.FileUtils;
@@ -45,6 +48,11 @@ import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Supplier;
 
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doCallRealMethod;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 public class TestSegmentedRaftLog extends BaseTest {
   static {
     LogUtils.setLogLevel(RaftLogWorker.LOG, Level.DEBUG);
@@ -175,7 +183,7 @@ public class TestSegmentedRaftLog extends BaseTest {
             new SimpleOperation("m" + index) :
             new SimpleOperation(stringSupplier.get());
         eList.add(ProtoUtils.toLogEntryProto(m.getLogEntryContent(),
-            range.term, index, clientId, callId));
+            range.term, index, clientId, index));
       }
     }
     return eList;
@@ -302,6 +310,16 @@ public class TestSegmentedRaftLog extends BaseTest {
     }
   }
 
+  private void checkFailedEntries(List<LogEntryProto> entries, long fromIndex, 
RetryCache retryCache) {
+    for (int i = 0; i < entries.size(); i++) {
+      if (i < fromIndex) {
+        RetryCacheTestUtil.assertFailure(retryCache, entries.get(i), false);
+      } else {
+        RetryCacheTestUtil.assertFailure(retryCache, entries.get(i), true);
+      }
+    }
+  }
+
   /**
    * Test append with inconsistent entries
    */
@@ -311,9 +329,14 @@ public class TestSegmentedRaftLog extends BaseTest {
     List<SegmentRange> ranges = prepareRanges(5, 200, 0);
     List<LogEntryProto> entries = prepareLogEntries(ranges, null);
 
+    RaftServerImpl server = mock(RaftServerImpl.class);
+    RetryCache retryCache = RetryCacheTestUtil.createRetryCache();
+    when(server.getRetryCache()).thenReturn(retryCache);
+    
doCallRealMethod().when(server).failClientRequest(any(LogEntryProto.class));
     try (SegmentedRaftLog raftLog =
-             new SegmentedRaftLog(peerId, null, storage, -1, properties)) {
+             new SegmentedRaftLog(peerId, server, storage, -1, properties)) {
       raftLog.open(RaftServerConstants.INVALID_LOG_INDEX, null);
+      entries.stream().forEach(entry -> 
RetryCacheTestUtil.createEntry(retryCache, entry));
       // append entries to the raftlog
       
entries.stream().map(raftLog::appendEntry).forEach(CompletableFuture::join);
     }
@@ -327,10 +350,11 @@ public class TestSegmentedRaftLog extends BaseTest {
         Arrays.asList(r1, r2, r3), null);
 
     try (SegmentedRaftLog raftLog =
-             new SegmentedRaftLog(peerId, null, storage, -1, properties)) {
+             new SegmentedRaftLog(peerId, server, storage, -1, properties)) {
       raftLog.open(RaftServerConstants.INVALID_LOG_INDEX, null);
       raftLog.append(newEntries.toArray(new 
LogEntryProto[newEntries.size()])).forEach(CompletableFuture::join);
 
+      checkFailedEntries(entries, 650, retryCache);
       checkEntries(raftLog, entries, 0, 650);
       checkEntries(raftLog, newEntries, 100, 100);
       Assert.assertEquals(newEntries.get(newEntries.size() - 1),
@@ -341,7 +365,7 @@ public class TestSegmentedRaftLog extends BaseTest {
 
     // load the raftlog again and check
     try (SegmentedRaftLog raftLog =
-             new SegmentedRaftLog(peerId, null, storage, -1, properties)) {
+             new SegmentedRaftLog(peerId, server, storage, -1, properties)) {
       raftLog.open(RaftServerConstants.INVALID_LOG_INDEX, null);
       checkEntries(raftLog, entries, 0, 650);
       checkEntries(raftLog, newEntries, 100, 100);

Reply via email to