Server code cleanup: remove rarely used methods and reduce method visibility.
Project: http://git-wip-us.apache.org/repos/asf/incubator-ratis/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ratis/commit/eaadf8e6 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ratis/tree/eaadf8e6 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ratis/diff/eaadf8e6 Branch: refs/heads/master Commit: eaadf8e66a98e2bc5e254af96a56f6d8f6d9800f Parents: a38e2f7 Author: Tsz-Wo Nicholas Sze <szets...@hortonworks.com> Authored: Tue Jan 3 19:42:48 2017 +0800 Committer: Tsz-Wo Nicholas Sze <szets...@hortonworks.com> Committed: Tue Jan 3 19:42:48 2017 +0800 ---------------------------------------------------------------------- .../apache/raft/server/impl/FollowerState.java | 4 +- .../apache/raft/server/impl/LeaderElection.java | 4 +- .../apache/raft/server/impl/LeaderState.java | 7 +-- .../raft/server/impl/RaftConfiguration.java | 12 ++--- .../apache/raft/server/impl/RaftServerImpl.java | 17 +++--- .../java/org/apache/raft/server/impl/Role.java | 25 --------- .../raft/server/impl/RaftServerTestUtil.java | 6 +++ .../raft/statemachine/RaftSnapshotBaseTest.java | 57 ++++++++++---------- .../SimpleStateMachine4Testing.java | 10 ++-- .../raft/statemachine/TestStateMachine.java | 11 ++-- 10 files changed, 71 insertions(+), 82 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/eaadf8e6/raft-server/src/main/java/org/apache/raft/server/impl/FollowerState.java ---------------------------------------------------------------------- diff --git a/raft-server/src/main/java/org/apache/raft/server/impl/FollowerState.java b/raft-server/src/main/java/org/apache/raft/server/impl/FollowerState.java index 20f2d8f..fbef188 100644 --- a/raft-server/src/main/java/org/apache/raft/server/impl/FollowerState.java +++ b/raft-server/src/main/java/org/apache/raft/server/impl/FollowerState.java @@ -67,8 +67,8 @@ class FollowerState extends Daemon { } synchronized (server) { if (!inLogSync && lastRpcTime.elapsedTimeMs() >= electionTimeout) { - LOG.info("{} changes to {}, lastRpcTime:{}, electionTimeout:{}", - server.getId(), Role.CANDIDATE, lastRpcTime, electionTimeout); + LOG.info("{} changes to CANDIDATE, lastRpcTime:{}, electionTimeout:{}ms", + server.getId(), lastRpcTime, electionTimeout); // election timeout, should become a candidate server.changeToCandidate(); break; http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/eaadf8e6/raft-server/src/main/java/org/apache/raft/server/impl/LeaderElection.java ---------------------------------------------------------------------- diff --git a/raft-server/src/main/java/org/apache/raft/server/impl/LeaderElection.java b/raft-server/src/main/java/org/apache/raft/server/impl/LeaderElection.java index 39bdb13..a326eb5 100644 --- a/raft-server/src/main/java/org/apache/raft/server/impl/LeaderElection.java +++ b/raft-server/src/main/java/org/apache/raft/server/impl/LeaderElection.java @@ -104,8 +104,8 @@ class LeaderElection extends Daemon { } catch (InterruptedException e) { // the leader election thread is interrupted. The peer may already step // down to a follower. The leader election should skip. - LOG.info("The leader election thread of peer {} is interrupted. " + - "Currently role: {}.", server.getId(), server.getRole()); + LOG.info(server.getId() + " " + getClass().getSimpleName() + + " thread is interrupted gracefully; server=" + server); } catch (IOException e) { LOG.warn("Failed to persist votedFor/term. Exit the leader election.", e); stopRunning(); http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/eaadf8e6/raft-server/src/main/java/org/apache/raft/server/impl/LeaderState.java ---------------------------------------------------------------------- diff --git a/raft-server/src/main/java/org/apache/raft/server/impl/LeaderState.java b/raft-server/src/main/java/org/apache/raft/server/impl/LeaderState.java index 39dc400..fbbcb85 100644 --- a/raft-server/src/main/java/org/apache/raft/server/impl/LeaderState.java +++ b/raft-server/src/main/java/org/apache/raft/server/impl/LeaderState.java @@ -308,11 +308,12 @@ public class LeaderState { } // the updated configuration does not need to be sync'ed here } catch (InterruptedException e) { + final String s = server.getId() + " " + getClass().getSimpleName() + + " thread is interrupted "; if (!running) { - LOG.info("The LeaderState gets is stopped"); + LOG.info(s + " gracefully; server=" + server); } else { - LOG.warn("The leader election thread of peer {} is interrupted. " - + "Currently role: {}.", server.getId(), server.getRole()); + LOG.warn(s + " UNEXPECTEDLY; server=" + server, e); throw new RuntimeException(e); } } catch (IOException e) { http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/eaadf8e6/raft-server/src/main/java/org/apache/raft/server/impl/RaftConfiguration.java ---------------------------------------------------------------------- diff --git a/raft-server/src/main/java/org/apache/raft/server/impl/RaftConfiguration.java b/raft-server/src/main/java/org/apache/raft/server/impl/RaftConfiguration.java index 28ff330..4879314 100644 --- a/raft-server/src/main/java/org/apache/raft/server/impl/RaftConfiguration.java +++ b/raft-server/src/main/java/org/apache/raft/server/impl/RaftConfiguration.java @@ -140,12 +140,12 @@ public class RaftConfiguration { } /** Is this configuration transitional, i.e. in the middle of a peer change? */ - public boolean isTransitional() { + boolean isTransitional() { return oldConf != null; } /** Is this configuration stable, i.e. no on-going peer change? */ - public boolean isStable() { + boolean isStable() { return oldConf == null; } @@ -157,7 +157,7 @@ public class RaftConfiguration { return oldConf != null && oldConf.contains(peerId); } - public boolean contains(String peerId) { + boolean contains(String peerId) { return containsInConf(peerId) && (oldConf == null || containsInOldConf(peerId)); } @@ -203,7 +203,7 @@ public class RaftConfiguration { } /** @return true if the self id together with the others are in the majority. */ - public boolean hasMajority(Collection<String> others, String selfId) { + boolean hasMajority(Collection<String> others, String selfId) { Preconditions.checkArgument(!others.contains(selfId)); return conf.hasMajority(others, selfId) && (oldConf == null || oldConf.hasMajority(others, selfId)); @@ -251,11 +251,11 @@ public class RaftConfiguration { return peers.get(index); } - public Collection<RaftPeer> getPeersInOldConf() { + Collection<RaftPeer> getPeersInOldConf() { return oldConf != null ? oldConf.getPeers() : Collections.emptyList(); } - public Collection<RaftPeer> getPeersInConf() { + Collection<RaftPeer> getPeersInConf() { return conf.getPeers(); } } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/eaadf8e6/raft-server/src/main/java/org/apache/raft/server/impl/RaftServerImpl.java ---------------------------------------------------------------------- diff --git a/raft-server/src/main/java/org/apache/raft/server/impl/RaftServerImpl.java b/raft-server/src/main/java/org/apache/raft/server/impl/RaftServerImpl.java index 1ea40f6..c8a2e28 100644 --- a/raft-server/src/main/java/org/apache/raft/server/impl/RaftServerImpl.java +++ b/raft-server/src/main/java/org/apache/raft/server/impl/RaftServerImpl.java @@ -58,6 +58,11 @@ public class RaftServerImpl implements RaftServer { static final String INSTALL_SNAPSHOT = CLASS_NAME + ".installSnapshot"; + /** Role of raft peer */ + enum Role { + LEADER, CANDIDATE, FOLLOWER + } + private final int minTimeoutMs; private final int maxTimeoutMs; @@ -97,19 +102,19 @@ public class RaftServerImpl implements RaftServer { appenderFactory = initAppenderFactory(); } - public int getMinTimeoutMs() { + int getMinTimeoutMs() { return minTimeoutMs; } - public int getMaxTimeoutMs() { + int getMaxTimeoutMs() { return maxTimeoutMs; } - public int getRandomTimeoutMs() { + int getRandomTimeoutMs() { return RaftUtils.getRandomBetween(minTimeoutMs, maxTimeoutMs); } - public StateMachine getStateMachine() { + StateMachine getStateMachine() { return this.stateMachine; } @@ -230,10 +235,6 @@ public class RaftServerImpl implements RaftServer { return role == Role.LEADER; } - Role getRole() { - return role; - } - /** * Change the server state to Follower if necessary * @param newTerm The new term. http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/eaadf8e6/raft-server/src/main/java/org/apache/raft/server/impl/Role.java ---------------------------------------------------------------------- diff --git a/raft-server/src/main/java/org/apache/raft/server/impl/Role.java b/raft-server/src/main/java/org/apache/raft/server/impl/Role.java deleted file mode 100644 index 1413961..0000000 --- a/raft-server/src/main/java/org/apache/raft/server/impl/Role.java +++ /dev/null @@ -1,25 +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.raft.server.impl; - -/** - * Role of Raft peer - */ -public enum Role { - LEADER, CANDIDATE, FOLLOWER -} http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/eaadf8e6/raft-server/src/test/java/org/apache/raft/server/impl/RaftServerTestUtil.java ---------------------------------------------------------------------- diff --git a/raft-server/src/test/java/org/apache/raft/server/impl/RaftServerTestUtil.java b/raft-server/src/test/java/org/apache/raft/server/impl/RaftServerTestUtil.java index 5103fca..bd1934f 100644 --- a/raft-server/src/test/java/org/apache/raft/server/impl/RaftServerTestUtil.java +++ b/raft-server/src/test/java/org/apache/raft/server/impl/RaftServerTestUtil.java @@ -20,6 +20,8 @@ package org.apache.raft.server.impl; import org.apache.raft.MiniRaftCluster; import org.apache.raft.RaftTestUtil; import org.apache.raft.protocol.RaftPeer; +import org.apache.raft.server.RaftServer; +import org.apache.raft.statemachine.StateMachine; import org.junit.Assert; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -64,4 +66,8 @@ public class RaftServerTestUtil { } Assert.assertEquals(peers.length, numIncluded + deadIncluded); } + + public static StateMachine getStateMachine(RaftServerImpl s) { + return s.getStateMachine(); + } } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/eaadf8e6/raft-server/src/test/java/org/apache/raft/statemachine/RaftSnapshotBaseTest.java ---------------------------------------------------------------------- diff --git a/raft-server/src/test/java/org/apache/raft/statemachine/RaftSnapshotBaseTest.java b/raft-server/src/test/java/org/apache/raft/statemachine/RaftSnapshotBaseTest.java index 721d12c..fbdcb8b 100644 --- a/raft-server/src/test/java/org/apache/raft/statemachine/RaftSnapshotBaseTest.java +++ b/raft-server/src/test/java/org/apache/raft/statemachine/RaftSnapshotBaseTest.java @@ -25,6 +25,7 @@ import org.apache.raft.client.RaftClient; import org.apache.raft.conf.RaftProperties; import org.apache.raft.protocol.RaftClientReply; import org.apache.raft.protocol.SetConfigurationRequest; +import org.apache.raft.server.RaftServer; import org.apache.raft.server.impl.RaftServerImpl; import org.apache.raft.server.impl.RaftServerTestUtil; import org.apache.raft.server.simulation.RequestHandler; @@ -61,6 +62,28 @@ public abstract class RaftSnapshotBaseTest { static final Logger LOG = LoggerFactory.getLogger(RaftSnapshotBaseTest.class); private static final int SNAPSHOT_TRIGGER_THRESHOLD = 10; + static File getSnapshotFile(MiniRaftCluster cluster, int i) { + final RaftServerImpl leader = cluster.getLeader(); + final SimpleStateMachine4Testing sm = SimpleStateMachine4Testing.get(leader); + return sm.getStateMachineStorage().getSnapshotFile( + leader.getState().getCurrentTerm(), i); + } + + static void assertLeaderContent(MiniRaftCluster cluster) + throws InterruptedException { + final RaftServerImpl leader = RaftTestUtil.waitForLeader(cluster); + Assert.assertEquals(SNAPSHOT_TRIGGER_THRESHOLD * 2, + leader.getState().getLog().getLastCommittedIndex()); + final LogEntryProto[] entries = SimpleStateMachine4Testing.get(leader).getContent(); + + for (int i = 1; i < SNAPSHOT_TRIGGER_THRESHOLD * 2 - 1; i++) { + Assert.assertEquals(i+1, entries[i].getIndex()); + Assert.assertArrayEquals( + new SimpleMessage("m" + i).getContent().toByteArray(), + entries[i].getSmLogEntry().getData().toByteArray()); + } + } + private MiniRaftCluster cluster; public abstract MiniRaftCluster initCluster(int numServer, RaftProperties prop) @@ -103,9 +126,7 @@ public abstract class RaftSnapshotBaseTest { } // wait for the snapshot to be done - StateMachine sm = cluster.getLeader().getStateMachine(); - File snapshotFile = ((SimpleStateMachineStorage)sm.getStateMachineStorage()) - .getSnapshotFile(cluster.getLeader().getState().getCurrentTerm(), i); + final File snapshotFile = getSnapshotFile(cluster, i); int retries = 0; do { @@ -117,19 +138,8 @@ public abstract class RaftSnapshotBaseTest { // restart the peer and check if it can correctly load snapshot cluster.restart(false); try { - RaftTestUtil.waitForLeader(cluster); - // 200 messages + two leader elections --> last committed = 201 - Assert.assertEquals(SNAPSHOT_TRIGGER_THRESHOLD * 2, - cluster.getLeader().getState().getLog().getLastCommittedIndex()); - sm = cluster.getLeader().getStateMachine(); - LogEntryProto[] entries = ((SimpleStateMachine4Testing) sm).getContent(); - for (i = 1; i < SNAPSHOT_TRIGGER_THRESHOLD * 2 - 1; i++) { - Assert.assertEquals(i+1, entries[i].getIndex()); - Assert.assertArrayEquals( - new SimpleMessage("m" + i).getContent().toByteArray(), - entries[i].getSmLogEntry().getData().toByteArray()); - } + assertLeaderContent(cluster); } finally { cluster.shutdown(); } @@ -158,9 +168,7 @@ public abstract class RaftSnapshotBaseTest { // wait for the snapshot to be done RaftStorageDirectory storageDirectory = cluster.getLeader().getState() .getStorage().getStorageDir(); - StateMachine sm = cluster.getLeader().getStateMachine(); - File snapshotFile = ((SimpleStateMachineStorage) sm.getStateMachineStorage()) - .getSnapshotFile(cluster.getLeader().getState().getCurrentTerm(), i); + final File snapshotFile = getSnapshotFile(cluster, i); logs = storageDirectory.getLogSegmentFiles(); int retries = 0; @@ -182,18 +190,7 @@ public abstract class RaftSnapshotBaseTest { LOG.info("Restarting the cluster"); cluster.restart(false); try { - RaftTestUtil.waitForLeader(cluster); - - Assert.assertEquals(SNAPSHOT_TRIGGER_THRESHOLD * 2, - cluster.getLeader().getState().getLog().getLastCommittedIndex()); - StateMachine sm = cluster.getLeader().getStateMachine(); - LogEntryProto[] entries = ((SimpleStateMachine4Testing) sm).getContent(); - for (int i = 1; i < SNAPSHOT_TRIGGER_THRESHOLD * 2 - 1; i++) { - Assert.assertEquals(i+1, entries[i].getIndex()); - Assert.assertArrayEquals( - new SimpleMessage("m" + i).getContent().toByteArray(), - entries[i].getSmLogEntry().getData().toByteArray()); - } + assertLeaderContent(cluster); // generate some more traffic try(final RaftClient client = cluster.createClient("client", http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/eaadf8e6/raft-server/src/test/java/org/apache/raft/statemachine/SimpleStateMachine4Testing.java ---------------------------------------------------------------------- diff --git a/raft-server/src/test/java/org/apache/raft/statemachine/SimpleStateMachine4Testing.java b/raft-server/src/test/java/org/apache/raft/statemachine/SimpleStateMachine4Testing.java index 227ea58..3e0ae15 100644 --- a/raft-server/src/test/java/org/apache/raft/statemachine/SimpleStateMachine4Testing.java +++ b/raft-server/src/test/java/org/apache/raft/statemachine/SimpleStateMachine4Testing.java @@ -17,7 +17,6 @@ */ package org.apache.raft.statemachine; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import org.apache.raft.RaftTestUtil.SimpleMessage; import org.apache.raft.conf.RaftProperties; @@ -26,6 +25,8 @@ import org.apache.raft.protocol.Message; import org.apache.raft.protocol.RaftClientReply; import org.apache.raft.protocol.RaftClientRequest; import org.apache.raft.server.impl.RaftServerConstants; +import org.apache.raft.server.impl.RaftServerImpl; +import org.apache.raft.server.impl.RaftServerTestUtil; import org.apache.raft.server.protocol.TermIndex; import org.apache.raft.server.storage.LogInputStream; import org.apache.raft.server.storage.LogOutputStream; @@ -60,6 +61,10 @@ public class SimpleStateMachine4Testing extends BaseStateMachine { = "raft.test.simple.state.machine.take.snapshot"; public static final boolean RAFT_TEST_SIMPLE_STATE_MACHINE_TAKE_SNAPSHOT_DEFAULT = false; + public static SimpleStateMachine4Testing get(RaftServerImpl s) { + return (SimpleStateMachine4Testing)RaftServerTestUtil.getStateMachine(s); + } + private final List<LogEntryProto> list = Collections.synchronizedList(new ArrayList<>()); private final Daemon checkpointer; @@ -174,7 +179,7 @@ public class SimpleStateMachine4Testing extends BaseStateMachine { } @Override - public StateMachineStorage getStateMachineStorage() { + public SimpleStateMachineStorage getStateMachineStorage() { return storage; } @@ -235,7 +240,6 @@ public class SimpleStateMachine4Testing extends BaseStateMachine { }); } - @VisibleForTesting public LogEntryProto[] getContent() { return list.toArray(new LogEntryProto[list.size()]); } http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/eaadf8e6/raft-server/src/test/java/org/apache/raft/statemachine/TestStateMachine.java ---------------------------------------------------------------------- diff --git a/raft-server/src/test/java/org/apache/raft/statemachine/TestStateMachine.java b/raft-server/src/test/java/org/apache/raft/statemachine/TestStateMachine.java index 5892c65..546bfb8 100644 --- a/raft-server/src/test/java/org/apache/raft/statemachine/TestStateMachine.java +++ b/raft-server/src/test/java/org/apache/raft/statemachine/TestStateMachine.java @@ -26,6 +26,7 @@ import org.apache.raft.protocol.Message; import org.apache.raft.protocol.RaftClientRequest; import org.apache.raft.server.RaftServerConfigKeys; import org.apache.raft.server.impl.RaftServerImpl; +import org.apache.raft.server.impl.RaftServerTestUtil; import org.apache.raft.server.simulation.MiniRaftClusterWithSimulatedRpc; import org.apache.raft.shaded.proto.RaftProtos.SMLogEntryProto; import org.apache.raft.util.RaftUtils; @@ -93,7 +94,11 @@ public class TestStateMachine { return properties; } - public static class SMTransactionContext extends SimpleStateMachine4Testing { + static class SMTransactionContext extends SimpleStateMachine4Testing { + public static SMTransactionContext get(RaftServerImpl s) { + return (SMTransactionContext)RaftServerTestUtil.getStateMachine(s); + } + AtomicReference<Throwable> throwable = new AtomicReference<>(null); AtomicLong transactions = new AtomicLong(0); AtomicBoolean isLeader = new AtomicBoolean(false); @@ -162,7 +167,7 @@ public class TestStateMachine { Thread.sleep(cluster.getMaxTimeout() + 100); for (RaftServerImpl raftServer : cluster.getServers()) { - SMTransactionContext sm = ((SMTransactionContext)raftServer.getStateMachine()); + final SMTransactionContext sm = SMTransactionContext.get(raftServer); sm.rethrowIfException(); assertEquals(numTrx, sm.numApplied.get()); } @@ -170,7 +175,7 @@ public class TestStateMachine { // check leader RaftServerImpl raftServer = cluster.getLeader(); // assert every transaction has obtained context in leader - SMTransactionContext sm = ((SMTransactionContext)raftServer.getStateMachine()); + final SMTransactionContext sm = SMTransactionContext.get(raftServer); List<Long> ll = sm.applied.stream().collect(Collectors.toList()); Collections.sort(ll); assertEquals(ll.toString(), ll.size(), numTrx);
