You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ratis.apache.org by ru...@apache.org on 2020/11/29 23:36:16 UTC
[incubator-ratis] branch master updated: RATIS-1187. Avoid directly
using RaftServerImpl in LeaderElectionMetrics. (#305)
This is an automated email from the ASF dual-hosted git repository.
runzhiwang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-ratis.git
The following commit(s) were added to refs/heads/master by this push:
new faa38be RATIS-1187. Avoid directly using RaftServerImpl in LeaderElectionMetrics. (#305)
faa38be is described below
commit faa38bee9495094de10b64f2e6149ea5476164fc
Author: Tsz-Wo Nicholas Sze <sz...@apache.org>
AuthorDate: Mon Nov 30 07:36:09 2020 +0800
RATIS-1187. Avoid directly using RaftServerImpl in LeaderElectionMetrics. (#305)
---
.../ratis/grpc/server/TestGrpcServerMetrics.java | 8 ------
.../org/apache/ratis/metrics/RatisMetrics.java | 2 +-
.../apache/ratis/server/impl/RaftServerImpl.java | 3 +-
.../org/apache/ratis/server/impl/ServerState.java | 5 ++--
.../server/metrics/LeaderElectionMetrics.java | 31 ++++++++++-----------
.../ratis/server/impl/LeaderElectionTests.java | 32 +++++++++++-----------
.../server/metrics/TestLeaderElectionMetrics.java | 13 ++-------
7 files changed, 38 insertions(+), 56 deletions(-)
diff --git a/ratis-grpc/src/test/java/org/apache/ratis/grpc/server/TestGrpcServerMetrics.java b/ratis-grpc/src/test/java/org/apache/ratis/grpc/server/TestGrpcServerMetrics.java
index cf2f60b..bc54300 100644
--- a/ratis-grpc/src/test/java/org/apache/ratis/grpc/server/TestGrpcServerMetrics.java
+++ b/ratis-grpc/src/test/java/org/apache/ratis/grpc/server/TestGrpcServerMetrics.java
@@ -25,7 +25,6 @@ import static org.apache.ratis.grpc.metrics.GrpcServerMetrics.RATIS_GRPC_METRICS
import static org.apache.ratis.grpc.metrics.GrpcServerMetrics.RATIS_GRPC_METRICS_LOG_APPENDER_TIMEOUT;
import static org.apache.ratis.grpc.metrics.GrpcServerMetrics.RATIS_GRPC_METRICS_REQUESTS_TOTAL;
import static org.apache.ratis.grpc.metrics.GrpcServerMetrics.RATIS_GRPC_METRICS_REQUEST_RETRY_COUNT;
-import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.SortedMap;
@@ -38,8 +37,6 @@ import org.apache.ratis.proto.RaftProtos;
import org.apache.ratis.protocol.RaftGroupId;
import org.apache.ratis.protocol.RaftGroupMemberId;
import org.apache.ratis.protocol.RaftPeerId;
-import org.apache.ratis.server.impl.RaftServerImpl;
-import org.apache.ratis.server.impl.ServerState;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -54,15 +51,10 @@ public class TestGrpcServerMetrics {
@BeforeClass
public static void setUp() throws Exception {
- RaftServerImpl raftServer = mock(RaftServerImpl.class);
- ServerState serverStateMock = mock(ServerState.class);
- when(raftServer.getState()).thenReturn(serverStateMock);
- when(serverStateMock.getLastLeaderElapsedTimeMs()).thenReturn(1000L);
raftGroupId = RaftGroupId.randomId();
raftPeerId = RaftPeerId.valueOf("TestId");
followerId = RaftPeerId.valueOf("FollowerId");
RaftGroupMemberId raftGroupMemberId = RaftGroupMemberId.valueOf(raftPeerId, raftGroupId);
- when(raftServer.getMemberId()).thenReturn(raftGroupMemberId);
grpcServerMetrics = new GrpcServerMetrics(raftGroupMemberId.toString());
ratisMetricRegistry = grpcServerMetrics.getRegistry();
}
diff --git a/ratis-metrics/src/main/java/org/apache/ratis/metrics/RatisMetrics.java b/ratis-metrics/src/main/java/org/apache/ratis/metrics/RatisMetrics.java
index 4806547..8c47fd2 100644
--- a/ratis-metrics/src/main/java/org/apache/ratis/metrics/RatisMetrics.java
+++ b/ratis-metrics/src/main/java/org/apache/ratis/metrics/RatisMetrics.java
@@ -30,7 +30,7 @@ public class RatisMetrics {
@SuppressWarnings("VisibilityModifier")
protected RatisMetricRegistry registry;
- protected RatisMetricRegistry create(MetricRegistryInfo info) {
+ protected static RatisMetricRegistry create(MetricRegistryInfo info) {
Optional<RatisMetricRegistry> metricRegistry = MetricRegistries.global().get(info);
return metricRegistry.orElseGet(() -> {
LOG.info("Creating Metrics Registry : {}", info.getName());
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 1393b0e..78ab013 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
@@ -140,7 +140,8 @@ public class RaftServerImpl implements RaftServer.Division,
this.dataStreamMap = new DataStreamMapImpl(id);
this.jmxAdapter = new RaftServerJmxAdapter();
- this.leaderElectionMetrics = LeaderElectionMetrics.getLeaderElectionMetrics(this);
+ this.leaderElectionMetrics = LeaderElectionMetrics.getLeaderElectionMetrics(
+ getMemberId(), state::getLastLeaderElapsedTimeMs);
this.raftServerMetrics = RaftServerMetrics.computeIfAbsentRaftServerMetrics(
getMemberId(), () -> commitInfoCache::get, () -> retryCache);
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java
index e2a495f..f1117de 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java
@@ -266,9 +266,8 @@ public class ServerState implements Closeable {
.isPresent();
}
- public long getLastLeaderElapsedTimeMs() {
- final Timestamp t = lastNoLeaderTime;
- return t == null ? 0 : t.elapsedTimeMs();
+ long getLastLeaderElapsedTimeMs() {
+ return Optional.ofNullable(lastNoLeaderTime).map(Timestamp::elapsedTimeMs).orElse(0L);
}
void becomeLeader() {
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/metrics/LeaderElectionMetrics.java b/ratis-server/src/main/java/org/apache/ratis/server/metrics/LeaderElectionMetrics.java
index 9bb4280..665f59b 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/metrics/LeaderElectionMetrics.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/metrics/LeaderElectionMetrics.java
@@ -21,11 +21,14 @@ package org.apache.ratis.server.metrics;
import org.apache.ratis.metrics.MetricRegistryInfo;
import org.apache.ratis.metrics.RatisMetricRegistry;
import org.apache.ratis.metrics.RatisMetrics;
-import org.apache.ratis.server.impl.RaftServerImpl;
+import org.apache.ratis.protocol.RaftGroupMemberId;
import org.apache.ratis.util.Timestamp;
import com.codahale.metrics.Timer;
+import java.util.Optional;
+import java.util.function.LongSupplier;
+
/**
* Class to update the metrics related to Leader Election.
*/
@@ -41,28 +44,24 @@ public final class LeaderElectionMetrics extends RatisMetrics {
public static final String LAST_LEADER_ELECTION_ELAPSED_TIME =
"lastLeaderElectionElapsedTime";
- private Timestamp lastElectionTime;
+ private volatile Timestamp lastElectionTime;
- private LeaderElectionMetrics(RaftServerImpl raftServer) {
- this.registry = getMetricRegistryForLeaderElection(raftServer.getMemberId().toString());
- registry.gauge(LAST_LEADER_ELAPSED_TIME, () -> () -> raftServer.getState().getLastLeaderElapsedTimeMs());
- registry.gauge(LAST_LEADER_ELECTION_ELAPSED_TIME, () -> () -> {
- if (lastElectionTime == null) {
- return -1L;
- } else {
- return lastElectionTime.elapsedTimeMs();
- }
- });
+ private LeaderElectionMetrics(RaftGroupMemberId serverId, LongSupplier getLastLeaderElapsedTimeMs) {
+ this.registry = getMetricRegistryForLeaderElection(serverId);
+ registry.gauge(LAST_LEADER_ELAPSED_TIME, () -> getLastLeaderElapsedTimeMs::getAsLong);
+ registry.gauge(LAST_LEADER_ELECTION_ELAPSED_TIME,
+ () -> () -> Optional.ofNullable(lastElectionTime).map(Timestamp::elapsedTimeMs).orElse(-1L));
}
- private RatisMetricRegistry getMetricRegistryForLeaderElection(String serverId) {
- return create(new MetricRegistryInfo(serverId,
+ public static RatisMetricRegistry getMetricRegistryForLeaderElection(RaftGroupMemberId serverId) {
+ return create(new MetricRegistryInfo(serverId.toString(),
RATIS_APPLICATION_NAME_METRICS, RATIS_LEADER_ELECTION_METRICS,
RATIS_LEADER_ELECTION_METRICS_DESC));
}
- public static LeaderElectionMetrics getLeaderElectionMetrics(RaftServerImpl raftServer) {
- return new LeaderElectionMetrics(raftServer);
+ public static LeaderElectionMetrics getLeaderElectionMetrics(
+ RaftGroupMemberId serverId, LongSupplier getLastLeaderElapsedTimeMs) {
+ return new LeaderElectionMetrics(serverId, getLastLeaderElapsedTimeMs);
}
public void onNewLeaderElectionCompletion() {
diff --git a/ratis-server/src/test/java/org/apache/ratis/server/impl/LeaderElectionTests.java b/ratis-server/src/test/java/org/apache/ratis/server/impl/LeaderElectionTests.java
index 2988baa..c8da02f 100644
--- a/ratis-server/src/test/java/org/apache/ratis/server/impl/LeaderElectionTests.java
+++ b/ratis-server/src/test/java/org/apache/ratis/server/impl/LeaderElectionTests.java
@@ -28,6 +28,7 @@ import org.apache.ratis.protocol.RaftGroupId;
import org.apache.ratis.protocol.RaftGroupMemberId;
import org.apache.ratis.protocol.RaftPeerId;
import org.apache.ratis.server.RaftServer;
+import org.apache.ratis.server.RaftServerConfigKeys;
import org.apache.ratis.server.metrics.LeaderElectionMetrics;
import org.apache.ratis.server.raftlog.segmented.SegmentedRaftLogTestUtils;
import org.apache.ratis.util.ExitUtils;
@@ -103,21 +104,18 @@ public abstract class LeaderElectionTests<CLUSTER extends MiniRaftCluster>
}
void runTestLostMajorityHeartbeats(CLUSTER cluster) throws Exception {
- final RaftServerImpl leader = waitForLeader(cluster);
+ final TimeDuration maxTimeout = RaftServerConfigKeys.Rpc.timeoutMax(getProperties());
+ final RaftServer.Division leader = waitForLeader(cluster);
try {
isolate(cluster, leader.getId());
- Thread.sleep(leader.getMaxTimeoutMs());
- Thread.sleep(leader.getMaxTimeoutMs());
- final Optional<FollowerState> optional = leader.getRole().getFollowerState();
- Assert.assertTrue(optional.isPresent());
- final FollowerState followerState = optional.get();
- Assert.assertTrue(followerState.lostMajorityHeartbeatsRecently());
+ maxTimeout.sleep();
+ maxTimeout.sleep();
+ RaftServerTestUtil.assertLostMajorityHeartbeatsRecently(leader);
} finally {
deIsolate(cluster, leader.getId());
}
}
-
@Test
public void testEnforceLeader() throws Exception {
LOG.info("Running testEnforceLeader");
@@ -139,13 +137,13 @@ public abstract class LeaderElectionTests<CLUSTER extends MiniRaftCluster>
static void enforceLeader(MiniRaftCluster cluster, final String newLeader, Logger LOG) throws InterruptedException {
LOG.info(cluster.printServers());
for(int i = 0; !cluster.tryEnforceLeader(newLeader) && i < 10; i++) {
- RaftServerImpl currLeader = cluster.getLeader();
+ final RaftServer.Division currLeader = cluster.getLeader();
LOG.info("try enforcing leader to " + newLeader + " but " +
(currLeader == null ? "no leader for round " + i : "new leader is " + currLeader.getId()));
}
LOG.info(cluster.printServers());
- final RaftServerImpl leader = cluster.getLeader();
+ final RaftServer.Division leader = cluster.getLeader();
Assert.assertEquals(newLeader, leader.getId().toString());
}
@@ -162,7 +160,7 @@ public abstract class LeaderElectionTests<CLUSTER extends MiniRaftCluster>
i.next().start();
}
- final RaftServerImpl leader = waitForLeader(cluster);
+ final RaftServer.Division leader = waitForLeader(cluster);
final TimeDuration sleepTime = TimeDuration.valueOf(3, TimeUnit.SECONDS);
LOG.info("sleep " + sleepTime);
sleepTime.sleep();
@@ -182,7 +180,7 @@ public abstract class LeaderElectionTests<CLUSTER extends MiniRaftCluster>
try(final MiniRaftCluster cluster = newCluster(3)) {
cluster.start();
- final RaftServerImpl leader = waitForLeader(cluster);
+ final RaftServer.Division leader = waitForLeader(cluster);
try (RaftClient client = cluster.createClient(leader.getId())) {
client.io().send(new RaftTestUtil.SimpleMessage("message"));
Thread.sleep(1000);
@@ -217,9 +215,10 @@ public abstract class LeaderElectionTests<CLUSTER extends MiniRaftCluster>
Timestamp timestamp = Timestamp.currentTime();
final MiniRaftCluster cluster = newCluster(3);
cluster.start();
- RaftServerImpl leaderServer = waitForLeader(cluster);
+ final RaftServer.Division leaderServer = waitForLeader(cluster);
- RatisMetricRegistry ratisMetricRegistry = LeaderElectionMetrics.getLeaderElectionMetrics(leaderServer).getRegistry();
+ final RatisMetricRegistry ratisMetricRegistry = LeaderElectionMetrics.getMetricRegistryForLeaderElection(
+ leaderServer.getMemberId());
// Verify each metric individually.
long numLeaderElections = ratisMetricRegistry.counter(LEADER_ELECTION_COUNT_METRIC).getCount();
@@ -270,8 +269,9 @@ public abstract class LeaderElectionTests<CLUSTER extends MiniRaftCluster>
RaftServerImpl server = mock(RaftServerImpl.class);
when(server.isAlive()).thenReturn(alive);
when(server.isCandidate()).thenReturn(false);
- when(server.getMemberId()).thenReturn(RaftGroupMemberId.valueOf(RaftPeerId.valueOf("any"), RaftGroupId.randomId()));
- LeaderElectionMetrics leaderElectionMetrics = LeaderElectionMetrics.getLeaderElectionMetrics(server);
+ final RaftGroupMemberId memberId = RaftGroupMemberId.valueOf(RaftPeerId.valueOf("any"), RaftGroupId.randomId());
+ when(server.getMemberId()).thenReturn(memberId);
+ LeaderElectionMetrics leaderElectionMetrics = LeaderElectionMetrics.getLeaderElectionMetrics(memberId, () -> 0);
when(server.getLeaderElectionMetrics()).thenReturn(leaderElectionMetrics);
return server;
}
diff --git a/ratis-server/src/test/java/org/apache/ratis/server/metrics/TestLeaderElectionMetrics.java b/ratis-server/src/test/java/org/apache/ratis/server/metrics/TestLeaderElectionMetrics.java
index 9c712ca..6270784 100644
--- a/ratis-server/src/test/java/org/apache/ratis/server/metrics/TestLeaderElectionMetrics.java
+++ b/ratis-server/src/test/java/org/apache/ratis/server/metrics/TestLeaderElectionMetrics.java
@@ -22,15 +22,11 @@ import static org.apache.ratis.server.metrics.LeaderElectionMetrics.LAST_LEADER_
import static org.apache.ratis.server.metrics.LeaderElectionMetrics.LEADER_ELECTION_TIMEOUT_COUNT_METRIC;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
import org.apache.ratis.metrics.RatisMetricRegistry;
import org.apache.ratis.protocol.RaftGroupId;
import org.apache.ratis.protocol.RaftGroupMemberId;
import org.apache.ratis.protocol.RaftPeerId;
-import org.apache.ratis.server.impl.RaftServerImpl;
-import org.apache.ratis.server.impl.ServerState;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -43,16 +39,11 @@ public class TestLeaderElectionMetrics {
private static RatisMetricRegistry ratisMetricRegistry;
@BeforeClass
- public static void setUp() throws Exception {
- RaftServerImpl raftServer = mock(RaftServerImpl.class);
- ServerState serverStateMock = mock(ServerState.class);
- when(raftServer.getState()).thenReturn(serverStateMock);
- when(serverStateMock.getLastLeaderElapsedTimeMs()).thenReturn(1000L);
+ public static void setUp() {
RaftGroupId raftGroupId = RaftGroupId.randomId();
RaftPeerId raftPeerId = RaftPeerId.valueOf("TestId");
RaftGroupMemberId raftGroupMemberId = RaftGroupMemberId.valueOf(raftPeerId, raftGroupId);
- when(raftServer.getMemberId()).thenReturn(raftGroupMemberId);
- leaderElectionMetrics = LeaderElectionMetrics.getLeaderElectionMetrics(raftServer);
+ leaderElectionMetrics = LeaderElectionMetrics.getLeaderElectionMetrics(raftGroupMemberId, () -> 1000L);
ratisMetricRegistry = leaderElectionMetrics.getRegistry();
}