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 <[email protected]> Authored: Tue Sep 18 15:26:08 2018 +0530 Committer: Lokesh Jain <[email protected]> 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()));
