You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ratis.apache.org by lj...@apache.org on 2018/09/18 09:57:36 UTC

incubator-ratis git commit: RATIS-322. Make RaftGroup a value base class. Contributed by Tsz Wo Nicholas Sze.

Repository: incubator-ratis
Updated Branches:
  refs/heads/master eca35312c -> b1c6d6503


RATIS-322. Make RaftGroup a value base class. Contributed by Tsz Wo Nicholas Sze.


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

Branch: refs/heads/master
Commit: b1c6d650394b1113192e4e757d8eca81606d2bab
Parents: eca3531
Author: Lokesh Jain <lj...@apache.org>
Authored: Tue Sep 18 15:26:08 2018 +0530
Committer: Lokesh Jain <lj...@apache.org>
Committed: Tue Sep 18 15:26:08 2018 +0530

----------------------------------------------------------------------
 .../org/apache/ratis/protocol/RaftGroup.java    | 54 +++++++++++++++-----
 .../org/apache/ratis/protocol/RaftGroupId.java  |  5 ++
 .../java/org/apache/ratis/protocol/RaftId.java  |  3 +-
 .../java/org/apache/ratis/util/ProtoUtils.java  |  3 +-
 .../ratis/examples/arithmetic/cli/Client.java   |  2 +-
 .../ratis/examples/arithmetic/cli/Server.java   |  2 +-
 .../ratis/server/impl/RaftServerImpl.java       |  2 +-
 .../ratis/server/impl/RaftServerProxy.java      |  2 +-
 .../java/org/apache/ratis/MiniRaftCluster.java  |  6 +--
 .../org/apache/ratis/RaftExceptionBaseTest.java |  2 +-
 .../server/impl/GroupManagementBaseTest.java    |  8 +--
 11 files changed, 61 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroup.java
----------------------------------------------------------------------
diff --git a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroup.java b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroup.java
index c119e32..f8c9a14 100644
--- a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroup.java
+++ b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroup.java
@@ -17,40 +17,52 @@
  */
 package org.apache.ratis.protocol;
 
+import org.apache.ratis.util.Preconditions;
+
 import java.util.*;
 
 /**
- * Description of a raft group. It has a globally unique ID and a group of raft
- * peers.
+ * Description of a raft group, which has a unique {@link RaftGroupId} and a collection of {@link RaftPeer}.
+ *
+ * This is a value-based class.
  */
-public class RaftGroup {
-  private static RaftGroup EMPTY_GROUP = new RaftGroup(RaftGroupId.emptyGroupId());
+public final class RaftGroup {
+  private static RaftGroup EMPTY_GROUP = new RaftGroup();
 
   public static RaftGroup emptyGroup() {
     return EMPTY_GROUP;
   }
 
-  /** UTF-8 string as id */
+  /** @return a group with the given id and peers. */
+  public static RaftGroup valueOf(RaftGroupId groupId, RaftPeer... peers) {
+    return new RaftGroup(groupId, peers == null || peers.length == 0? Collections.emptyList(): Arrays.asList(peers));
+  }
+
+  /** @return a group with the given id and peers. */
+  public static RaftGroup valueOf(RaftGroupId groupId, Collection<RaftPeer> peers) {
+    return new RaftGroup(groupId, peers);
+  }
+
+  /** The group id */
   private final RaftGroupId groupId;
   /** The group of raft peers */
   private final Map<RaftPeerId, RaftPeer> peers;
 
-  public RaftGroup(RaftGroupId groupId) {
-    this(groupId, Collections.emptyList());
-  }
-
-  public RaftGroup(RaftGroupId groupId, RaftPeer... peers) {
-    this(groupId, Arrays.asList(peers));
+  private RaftGroup() {
+    this.groupId = RaftGroupId.emptyGroupId();
+    this.peers = Collections.emptyMap();
   }
 
-  public RaftGroup(RaftGroupId groupId, Collection<RaftPeer> peers) {
+  private RaftGroup(RaftGroupId groupId, Collection<RaftPeer> peers) {
     this.groupId = Objects.requireNonNull(groupId, "groupId == null");
+    Preconditions.assertTrue(!groupId.equals(EMPTY_GROUP.getGroupId()),
+        () -> "Group Id " + groupId + " is reserved for the empty group.");
 
     if (peers == null || peers.isEmpty()) {
       this.peers = Collections.emptyMap();
     } else {
       final Map<RaftPeerId, RaftPeer> map = new HashMap<>();
-      peers.stream().forEach(p -> map.put(p.getId(), p));
+      peers.forEach(p -> map.put(p.getId(), p));
       this.peers = Collections.unmodifiableMap(map);
     }
   }
@@ -69,6 +81,22 @@ public class RaftGroup {
   }
 
   @Override
+  public int hashCode() {
+    return groupId.hashCode();
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    } else if (!(obj instanceof RaftGroup)) {
+      return false;
+    }
+    final RaftGroup that = (RaftGroup)obj;
+    return this.groupId.equals(that.groupId) && this.peers.equals(that.peers);
+  }
+
+  @Override
   public String toString() {
     return groupId + ":" + peers.values();
   }

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroupId.java
----------------------------------------------------------------------
diff --git a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroupId.java b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroupId.java
index d6c2e83..f850bec 100644
--- a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroupId.java
+++ b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroupId.java
@@ -21,6 +21,11 @@ import org.apache.ratis.shaded.com.google.protobuf.ByteString;
 
 import java.util.UUID;
 
+/**
+ * The id of a raft group.
+ *
+ * This is a value-based class.
+ */
 public final class RaftGroupId extends RaftId {
   private static final RaftGroupId EMPTY_GROUP_ID = new RaftGroupId(new UUID(0L, 0L));
 

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/ratis-common/src/main/java/org/apache/ratis/protocol/RaftId.java
----------------------------------------------------------------------
diff --git a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftId.java b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftId.java
index 933776e..9d81b7a 100644
--- a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftId.java
+++ b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftId.java
@@ -26,8 +26,9 @@ import java.util.Objects;
 import java.util.UUID;
 import java.util.function.Supplier;
 
+/** Unique identifier implemented using {@link UUID}. */
 public abstract class RaftId {
-  public static final int BYTE_LENGTH = 16;
+  private static final int BYTE_LENGTH = 16;
 
   private static void checkLength(int length, String name) {
     Preconditions.assertTrue(length == BYTE_LENGTH,

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/ratis-common/src/main/java/org/apache/ratis/util/ProtoUtils.java
----------------------------------------------------------------------
diff --git a/ratis-common/src/main/java/org/apache/ratis/util/ProtoUtils.java b/ratis-common/src/main/java/org/apache/ratis/util/ProtoUtils.java
index 06b1346..df0fab7 100644
--- a/ratis-common/src/main/java/org/apache/ratis/util/ProtoUtils.java
+++ b/ratis-common/src/main/java/org/apache/ratis/util/ProtoUtils.java
@@ -116,8 +116,7 @@ public interface ProtoUtils {
   }
 
   static RaftGroup toRaftGroup(RaftGroupProto proto) {
-    return new RaftGroup(toRaftGroupId(proto.getGroupId()),
-        toRaftPeerArray(proto.getPeersList()));
+    return RaftGroup.valueOf(toRaftGroupId(proto.getGroupId()), toRaftPeerArray(proto.getPeersList()));
   }
 
   static RaftGroupProto.Builder toRaftGroupProtoBuilder(RaftGroup group) {

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/ratis-examples/src/main/java/org/apache/ratis/examples/arithmetic/cli/Client.java
----------------------------------------------------------------------
diff --git a/ratis-examples/src/main/java/org/apache/ratis/examples/arithmetic/cli/Client.java b/ratis-examples/src/main/java/org/apache/ratis/examples/arithmetic/cli/Client.java
index aa89b25..b83b385 100644
--- a/ratis-examples/src/main/java/org/apache/ratis/examples/arithmetic/cli/Client.java
+++ b/ratis-examples/src/main/java/org/apache/ratis/examples/arithmetic/cli/Client.java
@@ -38,7 +38,7 @@ public abstract class Client extends SubCommandBase {
   public void run() throws Exception {
     RaftProperties raftProperties = new RaftProperties();
 
-    RaftGroup raftGroup = new RaftGroup(RaftGroupId.valueOf(ByteString.copyFromUtf8(raftGroupId)),
+    final RaftGroup raftGroup = RaftGroup.valueOf(RaftGroupId.valueOf(ByteString.copyFromUtf8(raftGroupId)),
         parsePeers(peers));
 
     RaftClient.Builder builder =

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/ratis-examples/src/main/java/org/apache/ratis/examples/arithmetic/cli/Server.java
----------------------------------------------------------------------
diff --git a/ratis-examples/src/main/java/org/apache/ratis/examples/arithmetic/cli/Server.java b/ratis-examples/src/main/java/org/apache/ratis/examples/arithmetic/cli/Server.java
index be3b2bd..547fc0a 100644
--- a/ratis-examples/src/main/java/org/apache/ratis/examples/arithmetic/cli/Server.java
+++ b/ratis-examples/src/main/java/org/apache/ratis/examples/arithmetic/cli/Server.java
@@ -62,7 +62,7 @@ public class Server extends SubCommandBase {
     RaftServerConfigKeys.setStorageDir(properties, storageDir);
     StateMachine stateMachine = new ArithmeticStateMachine();
 
-    RaftGroup raftGroup = new RaftGroup(RaftGroupId.valueOf(ByteString.copyFromUtf8(raftGroupId)), peers);
+    final RaftGroup raftGroup = RaftGroup.valueOf(RaftGroupId.valueOf(ByteString.copyFromUtf8(raftGroupId)), peers);
     RaftServer raftServer = RaftServer.newBuilder()
         .setServerId(RaftPeerId.valueOf(id))
         .setStateMachine(stateMachine).setProperties(properties)

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/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 1428c90..66603a2 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
@@ -234,7 +234,7 @@ public class RaftServerImpl implements RaftServerProtocol, RaftServerAsynchronou
   }
 
   RaftGroup getGroup() {
-    return new RaftGroup(groupId, getRaftConf().getPeers());
+    return RaftGroup.valueOf(groupId, getRaftConf().getPeers());
   }
 
   void shutdown(boolean deleteDirectory) {

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java
----------------------------------------------------------------------
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java
index 0afd596..2a119b2 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java
@@ -171,7 +171,7 @@ public class RaftServerProxy implements RaftServer {
           try {
             final RaftGroupId groupId = RaftGroupId.valueOf(UUID.fromString(sub.getName()));
             if (group == null || !groupId.equals(group.getGroupId())) {
-              addGroup(new RaftGroup(groupId));
+              addGroup(RaftGroup.valueOf(groupId));
             }
           } catch(Throwable t) {
             LOG.warn(getId() + ": Failed to initialize the group directory " + sub.getAbsolutePath() + ".  Ignoring it", t);

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/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 7a1e83e..0740956 100644
--- a/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java
+++ b/ratis-server/src/test/java/org/apache/ratis/MiniRaftCluster.java
@@ -124,7 +124,7 @@ public abstract class MiniRaftCluster implements Closeable {
         .map(RaftPeerId::valueOf)
         .map(id -> new RaftPeer(id, NetUtils.createLocalServerAddress()))
         .toArray(RaftPeer[]::new);
-    return new RaftGroup(RaftGroupId.randomId(), peers);
+    return RaftGroup.valueOf(RaftGroupId.randomId(), peers);
   }
 
   private final Supplier<File> rootTestDir = JavaUtils.memoize(
@@ -306,7 +306,7 @@ public abstract class MiniRaftCluster implements Closeable {
     final RaftPeer[] np = newPeers.toArray(new RaftPeer[newPeers.size()]);
     newPeers.addAll(group.getPeers());
     RaftPeer[] p = newPeers.toArray(new RaftPeer[newPeers.size()]);
-    group = new RaftGroup(group.getGroupId(), p);
+    group = RaftGroup.valueOf(group.getGroupId(), p);
     return new PeerChanges(p, np, new RaftPeer[0]);
   }
 
@@ -340,7 +340,7 @@ public abstract class MiniRaftCluster implements Closeable {
       }
     }
     RaftPeer[] p = peers.toArray(new RaftPeer[peers.size()]);
-    group = new RaftGroup(group.getGroupId(), p);
+    group = RaftGroup.valueOf(group.getGroupId(), p);
     return new PeerChanges(p, new RaftPeer[0],
         removedPeers.toArray(new RaftPeer[removedPeers.size()]));
   }

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/ratis-server/src/test/java/org/apache/ratis/RaftExceptionBaseTest.java
----------------------------------------------------------------------
diff --git a/ratis-server/src/test/java/org/apache/ratis/RaftExceptionBaseTest.java b/ratis-server/src/test/java/org/apache/ratis/RaftExceptionBaseTest.java
index ee338ed..58e26b2 100644
--- a/ratis-server/src/test/java/org/apache/ratis/RaftExceptionBaseTest.java
+++ b/ratis-server/src/test/java/org/apache/ratis/RaftExceptionBaseTest.java
@@ -174,7 +174,7 @@ public abstract class RaftExceptionBaseTest<CLUSTER extends MiniRaftCluster>
     final RaftGroup clusterGroup = cluster.getGroup();
     Assert.assertEquals(NUM_PEERS, clusterGroup.getPeers().size());
 
-    final RaftGroup anotherGroup = new RaftGroup(RaftGroupId.randomId(), clusterGroup.getPeers());
+    final RaftGroup anotherGroup = RaftGroup.valueOf(RaftGroupId.randomId(), clusterGroup.getPeers());
     Assert.assertNotEquals(clusterGroup.getGroupId(), anotherGroup.getGroupId());
 
     // Create client using another group

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/b1c6d650/ratis-server/src/test/java/org/apache/ratis/server/impl/GroupManagementBaseTest.java
----------------------------------------------------------------------
diff --git a/ratis-server/src/test/java/org/apache/ratis/server/impl/GroupManagementBaseTest.java b/ratis-server/src/test/java/org/apache/ratis/server/impl/GroupManagementBaseTest.java
index 682f2cb..66e3c61 100644
--- a/ratis-server/src/test/java/org/apache/ratis/server/impl/GroupManagementBaseTest.java
+++ b/ratis-server/src/test/java/org/apache/ratis/server/impl/GroupManagementBaseTest.java
@@ -83,7 +83,7 @@ public abstract class GroupManagementBaseTest extends BaseTest {
     Assert.assertNull(cluster.getLeader());
 
     // Add groups
-    final RaftGroup newGroup = new RaftGroup(RaftGroupId.randomId(), cluster.getPeers());
+    final RaftGroup newGroup = RaftGroup.valueOf(RaftGroupId.randomId(), cluster.getPeers());
     LOG.info("add new group: " + newGroup);
     final RaftClient client = cluster.createClient(newGroup);
     for(RaftPeer p : newGroup.getPeers()) {
@@ -141,7 +141,7 @@ public abstract class GroupManagementBaseTest extends BaseTest {
     LOG.info("\n\nrunMultiGroupTest with " + type + ": " + cluster.printServers());
 
     // Start server with an empty conf
-    final RaftGroup emptyGroup = new RaftGroup(cluster.getGroupId());
+    final RaftGroup emptyGroup = RaftGroup.valueOf(cluster.getGroupId());
 
     final List<RaftPeerId> ids = Arrays.stream(MiniRaftCluster.generateIds(idIndex[idIndex.length - 1], 0))
         .map(RaftPeerId::valueOf).collect(Collectors.toList());
@@ -165,7 +165,7 @@ public abstract class GroupManagementBaseTest extends BaseTest {
       final RaftGroupId gid = RaftGroupId.randomId();
       final int previous = i == 0 ? 0 : idIndex[i - 1];
       final RaftPeer[] peers = allPeers.subList(previous, idIndex[i]).toArray(RaftPeer.emptyArray());
-      groups[i] = new RaftGroup(gid, peers);
+      groups[i] = RaftGroup.valueOf(gid, peers);
 
       LOG.info(i + ") starting " + groups[i]);
       for(RaftPeer p : peers) {
@@ -204,7 +204,7 @@ public abstract class GroupManagementBaseTest extends BaseTest {
     LOG.info("close groups: " + cluster.printServers());
 
     // update chosen group to use all the peers
-    final RaftGroup newGroup = new RaftGroup(groups[chosen].getGroupId());
+    final RaftGroup newGroup = RaftGroup.valueOf(groups[chosen].getGroupId());
     for(int i = 0; i < groups.length; i++) {
       if (i != chosen) {
         LOG.info(i + ") groupAdd: " + cluster.printServers(groups[i].getGroupId()));