You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ratis.apache.org by ji...@apache.org on 2017/09/04 21:52:56 UTC

incubator-ratis git commit: Revert "Ratis-105. Server should check group id for client requests. Contributed by Tsz Wo Nicholas Sze."

Repository: incubator-ratis
Updated Branches:
  refs/heads/master 4dbe99bbd -> c2032801a


Revert "Ratis-105. Server should check group id for client requests. Contributed by Tsz Wo Nicholas Sze."

This reverts commit 4dbe99bbd1c5fc7e2d9a6eba4d6df5726b68a9c8.


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

Branch: refs/heads/master
Commit: c2032801a21bbe518989af7796c3ab0e408d7ef0
Parents: 4dbe99b
Author: Jing Zhao <ji...@apache.org>
Authored: Mon Sep 4 14:52:37 2017 -0700
Committer: Jing Zhao <ji...@apache.org>
Committed: Mon Sep 4 14:52:37 2017 -0700

----------------------------------------------------------------------
 .../ratis/protocol/GroupMismatchException.java  | 28 --------
 .../org/apache/ratis/TestMultiRaftGroup.java    | 76 --------------------
 .../ratis/server/impl/PendingRequests.java      |  7 +-
 .../ratis/server/impl/RaftServerImpl.java       | 12 ----
 .../ratis/server/impl/StateMachineUpdater.java  |  4 --
 .../java/org/apache/ratis/MiniRaftCluster.java  |  4 --
 .../java/org/apache/ratis/RaftBasicTests.java   |  4 +-
 .../server/impl/ReinitializationBaseTest.java   | 36 +++-------
 .../ratis/statemachine/TestStateMachine.java    |  2 +-
 9 files changed, 16 insertions(+), 157 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/c2032801/ratis-common/src/main/java/org/apache/ratis/protocol/GroupMismatchException.java
----------------------------------------------------------------------
diff --git a/ratis-common/src/main/java/org/apache/ratis/protocol/GroupMismatchException.java b/ratis-common/src/main/java/org/apache/ratis/protocol/GroupMismatchException.java
deleted file mode 100644
index af60825..0000000
--- a/ratis-common/src/main/java/org/apache/ratis/protocol/GroupMismatchException.java
+++ /dev/null
@@ -1,28 +0,0 @@
-/**
- * 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.protocol;
-
-/**
- * This exception indicates that the group id in the request does not match
- * server's group id.
- */
-public class GroupMismatchException extends RaftException {
-  public GroupMismatchException(String message) {
-    super(message);
-  }
-}

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/c2032801/ratis-examples/src/test/java/org/apache/ratis/TestMultiRaftGroup.java
----------------------------------------------------------------------
diff --git a/ratis-examples/src/test/java/org/apache/ratis/TestMultiRaftGroup.java b/ratis-examples/src/test/java/org/apache/ratis/TestMultiRaftGroup.java
deleted file mode 100644
index 8d3966e..0000000
--- a/ratis-examples/src/test/java/org/apache/ratis/TestMultiRaftGroup.java
+++ /dev/null
@@ -1,76 +0,0 @@
-/**
- * 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;
-
-
-import org.apache.log4j.Level;
-import org.apache.ratis.client.RaftClient;
-import org.apache.ratis.examples.RaftExamplesTestUtil;
-import org.apache.ratis.examples.arithmetic.ArithmeticStateMachine;
-import org.apache.ratis.examples.arithmetic.TestArithmetic;
-import org.apache.ratis.protocol.RaftGroup;
-import org.apache.ratis.server.impl.RaftServerImpl;
-import org.apache.ratis.server.impl.ReinitializationBaseTest;
-import org.apache.ratis.util.CheckedBiConsumer;
-import org.apache.ratis.util.LogUtils;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-import java.io.IOException;
-import java.util.Collection;
-import java.util.concurrent.atomic.AtomicInteger;
-
-@RunWith(Parameterized.class)
-public class TestMultiRaftGroup extends BaseTest {
-  static {
-    LogUtils.setLogLevel(RaftServerImpl.LOG, Level.TRACE);
-  }
-
-  @Parameterized.Parameters
-  public static Collection<Object[]> data() throws IOException {
-    return RaftExamplesTestUtil.getMiniRaftClusters(ArithmeticStateMachine.class, 0);
-  }
-
-  @Parameterized.Parameter
-  public MiniRaftCluster cluster;
-
-  @Test
-  public void testMultiRaftGroup() throws Exception {
-    runTestMultiRaftGroup(3, 6, 9, 12, 15);
-  }
-
-  private void runTestMultiRaftGroup(int... idIndex) throws Exception {
-    runTestMultiRaftGroup(idIndex, -1);
-  }
-
-  private final AtomicInteger start = new AtomicInteger(3);
-  private final int count = 10;
-
-  private void runTestMultiRaftGroup(int[] idIndex, int chosen) throws Exception {
-
-    final CheckedBiConsumer<MiniRaftCluster, RaftGroup, IOException> checker = (cluster, group) -> {
-      try (final RaftClient client = cluster.createClient(group)) {
-        TestArithmetic.runTestPythagorean(client, start.getAndAdd(2*count), count);
-      }
-    };
-
-    ReinitializationBaseTest.runTestReinitializeMultiGroups(
-        cluster, idIndex, chosen, checker);
-  }
-}

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/c2032801/ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java
----------------------------------------------------------------------
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java
index b7b8a9e..98bc0a7 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java
@@ -24,6 +24,7 @@ import org.slf4j.Logger;
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.stream.Collectors;
@@ -44,10 +45,7 @@ class PendingRequests {
       TransactionContext entry) {
     // externally synced for now
     Preconditions.assertTrue(!request.isReadOnly());
-    if (last != null && !(last.getRequest() instanceof SetConfigurationRequest)) {
-      Preconditions.assertTrue(index == last.getIndex() + 1,
-          () -> "index = " + index + " != last.getIndex() + 1, last=" + last);
-    }
+    Preconditions.assertTrue(last == null || index == last.getIndex() + 1);
     return add(index, request, entry);
   }
 
@@ -62,7 +60,6 @@ class PendingRequests {
   PendingRequest addConfRequest(SetConfigurationRequest request) {
     Preconditions.assertTrue(pendingSetConf == null);
     pendingSetConf = new PendingRequest(request);
-    last = pendingSetConf;
     return pendingSetConf;
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/c2032801/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 4463914..14acfb4 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
@@ -398,14 +398,6 @@ public class RaftServerImpl implements RaftServerProtocol,
         expected);
   }
 
-  void assertGroup(Object requestorId, RaftGroupId requestorGroupId) throws GroupMismatchException {
-    if (!groupId.equals(requestorGroupId)) {
-      throw new GroupMismatchException(getId()
-          + ": The group (" + requestorGroupId + ") of requestor " + requestorId
-          + " does not match the group (" + groupId + ") of the server " + getId());
-    }
-  }
-
   /**
    * Handle a normal update request from client.
    */
@@ -603,7 +595,6 @@ public class RaftServerImpl implements RaftServerProtocol,
     LOG.debug("{}: receive requestVote({}, {}, {}, {})",
         getId(), candidateId, candidateGroupId, candidateTerm, candidateLastEntry);
     assertLifeCycleState(RUNNING);
-    assertGroup(candidateId, candidateGroupId);
 
     boolean voteGranted = false;
     boolean shouldShutdown = false;
@@ -708,7 +699,6 @@ public class RaftServerImpl implements RaftServerProtocol,
             + initializing + ServerProtoUtils.toString(entries));
 
     assertLifeCycleState(STARTING, RUNNING);
-    assertGroup(leaderId, leaderGroupId);
 
     try {
       validateEntries(leaderTerm, previous, entries);
@@ -802,13 +792,11 @@ public class RaftServerImpl implements RaftServerProtocol,
       InstallSnapshotRequestProto request) throws IOException {
     final RaftRpcRequestProto r = request.getServerRequest();
     final RaftPeerId leaderId = RaftPeerId.valueOf(r.getRequestorId());
-    final RaftGroupId leaderGroupId = ProtoUtils.toRaftGroupId(r.getRaftGroupId());
     CodeInjectionForTesting.execute(INSTALL_SNAPSHOT, getId(),
         leaderId, request);
     LOG.debug("{}: receive installSnapshot({})", getId(), request);
 
     assertLifeCycleState(STARTING, RUNNING);
-    assertGroup(leaderId, leaderGroupId);
 
     final long currentTerm;
     final long leaderTerm = request.getLeaderTerm();

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/c2032801/ratis-server/src/main/java/org/apache/ratis/server/impl/StateMachineUpdater.java
----------------------------------------------------------------------
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/StateMachineUpdater.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/StateMachineUpdater.java
index 9ef6ce7..7ed6731 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/StateMachineUpdater.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/StateMachineUpdater.java
@@ -86,10 +86,6 @@ class StateMachineUpdater implements Runnable {
     state = State.STOP;
     updater.interrupt();
     try {
-      updater.join();
-    } catch (InterruptedException ignored) {
-    }
-    try {
       stateMachine.close();
     } catch (IOException ignored) {
     }

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/c2032801/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java
----------------------------------------------------------------------
diff --git a/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java b/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java
index 79cb9bb..b527a58 100644
--- a/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java
+++ b/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java
@@ -422,10 +422,6 @@ public abstract class MiniRaftCluster {
     return createClient(null, group);
   }
 
-  public RaftClient createClient(RaftGroup g) {
-    return createClient(null, g);
-  }
-
   public RaftClient createClient(RaftPeerId leaderId) {
     return createClient(leaderId, group);
   }

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/c2032801/ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java
----------------------------------------------------------------------
diff --git a/ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java b/ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java
index 7e08809..b227e47 100644
--- a/ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java
+++ b/ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java
@@ -80,7 +80,7 @@ public abstract class RaftBasicTests extends BaseTest {
     LOG.info(cluster.printServers());
 
     final SimpleMessage[] messages = SimpleMessage.create(10);
-    try(final RaftClient client = cluster.createClient()) {
+    try(final RaftClient client = cluster.createClient(null)) {
       for (SimpleMessage message : messages) {
         client.send(message);
       }
@@ -149,7 +149,7 @@ public abstract class RaftBasicTests extends BaseTest {
 
     final List<Client4TestWithLoad> clients
         = Stream.iterate(0, i -> i+1).limit(numClients)
-        .map(i -> cluster.createClient())
+        .map(i -> cluster.createClient(null))
         .map(c -> new Client4TestWithLoad(c, numMessages))
         .collect(Collectors.toList());
     clients.forEach(Thread::start);

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/c2032801/ratis-server/src/test/java/org/apache/ratis/server/impl/ReinitializationBaseTest.java
----------------------------------------------------------------------
diff --git a/ratis-server/src/test/java/org/apache/ratis/server/impl/ReinitializationBaseTest.java b/ratis-server/src/test/java/org/apache/ratis/server/impl/ReinitializationBaseTest.java
index d9068d1..5bc8dbe 100644
--- a/ratis-server/src/test/java/org/apache/ratis/server/impl/ReinitializationBaseTest.java
+++ b/ratis-server/src/test/java/org/apache/ratis/server/impl/ReinitializationBaseTest.java
@@ -27,13 +27,10 @@ import org.apache.ratis.protocol.RaftGroup;
 import org.apache.ratis.protocol.RaftGroupId;
 import org.apache.ratis.protocol.RaftPeer;
 import org.apache.ratis.protocol.RaftPeerId;
-import org.apache.ratis.util.CheckedBiConsumer;
 import org.apache.ratis.util.JavaUtils;
 import org.apache.ratis.util.LogUtils;
 import org.junit.Assert;
 import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -45,9 +42,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
 public abstract class ReinitializationBaseTest extends BaseTest {
-  static final Logger LOG = LoggerFactory.getLogger(ReinitializationBaseTest.class);
-
-  {
+  static {
     LogUtils.setLogLevel(RaftServerImpl.LOG, Level.DEBUG);
     LogUtils.setLogLevel(RaftClient.LOG, Level.DEBUG);
   }
@@ -83,7 +78,7 @@ public abstract class ReinitializationBaseTest extends BaseTest {
 
     // Reinitialize servers
     final RaftGroup newGroup = new RaftGroup(groupId, cluster.getPeers());
-    final RaftClient client = cluster.createClient(newGroup);
+    final RaftClient client = cluster.createClient(null, newGroup);
     for(RaftPeer p : newGroup.getPeers()) {
       client.reinitialize(newGroup, p.getId());
     }
@@ -111,15 +106,8 @@ public abstract class ReinitializationBaseTest extends BaseTest {
 
   private void runTestReinitializeMultiGroups(int[] idIndex, int chosen) throws Exception {
     printThreadCount(null, "init");
-    runTestReinitializeMultiGroups(getCluster(0), idIndex, chosen, NOOP);
-  }
-
-  static final CheckedBiConsumer<MiniRaftCluster, RaftGroup, RuntimeException> NOOP = (c, g) -> {};
+    final MiniRaftCluster cluster = getCluster(0);
 
-  public static <T extends Throwable> void runTestReinitializeMultiGroups(
-      MiniRaftCluster cluster, int[] idIndex, int chosen,
-      CheckedBiConsumer<MiniRaftCluster, RaftGroup, T> checker)
-      throws IOException, InterruptedException, T {
     if (chosen < 0) {
       chosen = ThreadLocalRandom.current().nextInt(idIndex.length);
     }
@@ -159,7 +147,6 @@ public abstract class ReinitializationBaseTest extends BaseTest {
         }
       }
       Assert.assertNotNull(RaftTestUtil.waitForLeader(cluster, true, gid));
-      checker.accept(cluster, groups[i]);
     }
     printThreadCount(type, "start groups");
     LOG.info("start groups: " + cluster.printServers());
@@ -184,23 +171,22 @@ public abstract class ReinitializationBaseTest extends BaseTest {
 
     // update chosen group to use all the peers
     final RaftGroup newGroup = new RaftGroup(groups[chosen].getGroupId());
+    final RaftPeer[] array = allPeers.toArray(RaftPeer.EMPTY_PEERS);
     for(int i = 0; i < groups.length; i++) {
-      if (i != chosen) {
-        LOG.info(i + ") reinitialize: " + cluster.printServers(groups[i].getGroupId()));
-        for (RaftPeer p : groups[i].getPeers()) {
+      LOG.info(i + ") update " + cluster.printServers(groups[i].getGroupId()));
+      if (i == chosen) {
+        try (final RaftClient client = cluster.createClient(null, groups[i])) {
+          client.setConfiguration(array);
+        }
+      } else {
+        for(RaftPeer p : groups[i].getPeers()) {
           try (final RaftClient client = cluster.createClient(p.getId(), groups[i])) {
             client.reinitialize(newGroup, p.getId());
           }
         }
       }
     }
-    LOG.info(chosen + ") setConfiguration: " + cluster.printServers(groups[chosen].getGroupId()));
-    try (final RaftClient client = cluster.createClient(groups[chosen])) {
-      client.setConfiguration(allPeers.toArray(RaftPeer.EMPTY_PEERS));
-    }
-
     Assert.assertNotNull(RaftTestUtil.waitForLeader(cluster, true));
-    checker.accept(cluster, groups[chosen]);
     LOG.info("update groups: " + cluster.printServers());
     printThreadCount(type, "update groups");
 

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/c2032801/ratis-server/src/test/java/org/apache/ratis/statemachine/TestStateMachine.java
----------------------------------------------------------------------
diff --git a/ratis-server/src/test/java/org/apache/ratis/statemachine/TestStateMachine.java b/ratis-server/src/test/java/org/apache/ratis/statemachine/TestStateMachine.java
index 73ce69d..f41e764 100644
--- a/ratis-server/src/test/java/org/apache/ratis/statemachine/TestStateMachine.java
+++ b/ratis-server/src/test/java/org/apache/ratis/statemachine/TestStateMachine.java
@@ -156,7 +156,7 @@ public class TestStateMachine extends BaseTest {
 
     int numTrx = 100;
     final RaftTestUtil.SimpleMessage[] messages = RaftTestUtil.SimpleMessage.create(numTrx);
-    try(final RaftClient client = cluster.createClient()) {
+    try(final RaftClient client = cluster.createClient(null)) {
       for (RaftTestUtil.SimpleMessage message : messages) {
         client.send(message);
       }