You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ratis.apache.org by sz...@apache.org on 2019/03/28 06:10:46 UTC
[incubator-ratis] branch master updated: RATIS-506.
ServerRestartTests.testRestartCommitIndex may fail intermittently.
This is an automated email from the ASF dual-hosted git repository.
szetszwo 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 97c152b RATIS-506. ServerRestartTests.testRestartCommitIndex may fail intermittently.
97c152b is described below
commit 97c152bb2fbabf462d32e6ebb05dfb30f4ed9a18
Author: Tsz Wo Nicholas Sze <sz...@apache.org>
AuthorDate: Thu Mar 28 14:10:08 2019 +0800
RATIS-506. ServerRestartTests.testRestartCommitIndex may fail intermittently.
---
.../main/java/org/apache/ratis/util/JavaUtils.java | 8 ---
.../ratis/statemachine/impl/BaseStateMachine.java | 2 +-
.../apache/ratis/server/ServerRestartTests.java | 36 ++++++----
.../server/impl/RaftReconfigurationBaseTest.java | 2 +-
.../ratis/statemachine/RaftSnapshotBaseTest.java | 2 +-
.../ratis/server/storage/TestRaftLogIndex.java | 84 ++++++++++++++++++++++
6 files changed, 109 insertions(+), 25 deletions(-)
diff --git a/ratis-common/src/main/java/org/apache/ratis/util/JavaUtils.java b/ratis-common/src/main/java/org/apache/ratis/util/JavaUtils.java
index d004c3f..94e2640 100644
--- a/ratis-common/src/main/java/org/apache/ratis/util/JavaUtils.java
+++ b/ratis-common/src/main/java/org/apache/ratis/util/JavaUtils.java
@@ -180,14 +180,6 @@ public interface JavaUtils {
attempt(CheckedRunnable.asCheckedSupplier(runnable), numAttempts, sleepTime, name, log);
}
- /** @deprecated use {@link #attempt(BooleanSupplier, int, TimeDuration, String, Logger)} */
- @Deprecated
- static void attempt(
- BooleanSupplier condition, int numAttempts, long sleepMs, String name, Logger log)
- throws InterruptedException {
- attempt(condition, numAttempts, TimeDuration.valueOf(sleepMs, TimeUnit.MILLISECONDS), name, log);
- }
-
/** Attempt to wait the given condition to return true multiple times. */
static void attempt(
BooleanSupplier condition, int numAttempts, TimeDuration sleepTime, String name, Logger log)
diff --git a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/BaseStateMachine.java b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/BaseStateMachine.java
index f038b86..8a09bd4 100644
--- a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/BaseStateMachine.java
+++ b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/BaseStateMachine.java
@@ -103,7 +103,7 @@ public class BaseStateMachine implements StateMachine {
return lastAppliedTermIndex.get();
}
- public void setLastAppliedTermIndex(TermIndex newTI) {
+ protected void setLastAppliedTermIndex(TermIndex newTI) {
lastAppliedTermIndex.set(newTI);
}
diff --git a/ratis-server/src/test/java/org/apache/ratis/server/ServerRestartTests.java b/ratis-server/src/test/java/org/apache/ratis/server/ServerRestartTests.java
index ced1aba..3b18ade 100644
--- a/ratis-server/src/test/java/org/apache/ratis/server/ServerRestartTests.java
+++ b/ratis-server/src/test/java/org/apache/ratis/server/ServerRestartTests.java
@@ -1,4 +1,4 @@
-/**
+/*
* 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
@@ -34,6 +34,7 @@ import org.apache.ratis.server.impl.ServerProtoUtils;
import org.apache.ratis.server.impl.ServerState;
import org.apache.ratis.server.protocol.TermIndex;
import org.apache.ratis.server.storage.RaftLog;
+import org.apache.ratis.server.storage.RaftLogIOException;
import org.apache.ratis.server.storage.RaftStorageDirectory.LogPathAndIndex;
import org.apache.ratis.server.storage.SegmentedRaftLogFormat;
import org.apache.ratis.statemachine.SimpleStateMachine4Testing;
@@ -107,7 +108,7 @@ public abstract class ServerRestartTests<CLUSTER extends MiniRaftCluster>
// make sure the restarted follower can catchup
final ServerState followerState = cluster.getRaftServerImpl(followerId).getState();
JavaUtils.attempt(() -> followerState.getLastAppliedIndex() >= leaderLastIndex,
- 10, 500, "follower catchup", LOG);
+ 10, ONE_SECOND, "follower catchup", LOG);
// make sure the restarted peer's log segments is correct
final RaftServerImpl follower = cluster.restartServer(followerId, false);
@@ -250,30 +251,21 @@ public abstract class ServerRestartTests<CLUSTER extends MiniRaftCluster>
RaftTestUtil.getStateMachineLogEntries(leaderLog);
- // check that the last logged commit index is equal to the index of the last committed StateMachineLogEntry
+ // check that the last metadata entry is written to the log
+ JavaUtils.attempt(() -> assertLastLogEntry(leader), 20, HUNDRED_MILLIS, "leader last metadata entry", LOG);
+
final long lastIndex = leaderLog.getLastEntryTermIndex().getIndex();
LOG.info("{}: leader lastIndex={}", leaderId, lastIndex);
- JavaUtils.attempt(() -> leaderLog.getLastCommittedIndex() == lastIndex,
- 10, HUNDRED_MILLIS, "leader(commitIndex == lastIndex)", LOG);
-
final LogEntryProto lastEntry = leaderLog.get(lastIndex);
LOG.info("{}: leader lastEntry entry[{}] = {}", leaderId, lastIndex, ServerProtoUtils.toLogEntryString(lastEntry));
- Assert.assertTrue(lastEntry.hasMetadataEntry());
final long loggedCommitIndex = lastEntry.getMetadataEntry().getCommitIndex();
- for(long i = lastIndex - 1; i > loggedCommitIndex; i--) {
- final LogEntryProto entry = leaderLog.get(i);
- LOG.info("{}: leader entry[{}] = {}", leaderId, i, ServerProtoUtils.toLogEntryString(entry));
- }
final LogEntryProto lastCommittedEntry = leaderLog.get(loggedCommitIndex);
LOG.info("{}: leader lastCommittedEntry = entry[{}] = {}",
leaderId, loggedCommitIndex, ServerProtoUtils.toLogEntryString(lastCommittedEntry));
- Assert.assertTrue(lastCommittedEntry.hasStateMachineLogEntry());
final SimpleStateMachine4Testing leaderStateMachine = SimpleStateMachine4Testing.get(leader);
final TermIndex lastAppliedTermIndex = leaderStateMachine.getLastAppliedTermIndex();
LOG.info("{}: leader lastAppliedTermIndex = {}", leaderId, lastAppliedTermIndex);
- Assert.assertEquals(lastCommittedEntry.getTerm(), lastAppliedTermIndex.getTerm());
- Assert.assertEquals(lastCommittedEntry.getIndex(), lastAppliedTermIndex.getIndex());
// check follower logs
for(RaftServerImpl s : cluster.iterateServerImpls()) {
@@ -304,4 +296,20 @@ public abstract class ServerRestartTests<CLUSTER extends MiniRaftCluster>
cluster.killServer(id);
}
}
+
+ static void assertLastLogEntry(RaftServerImpl server) throws RaftLogIOException {
+ final RaftLog raftLog = server.getState().getLog();
+ final long lastIndex = raftLog.getLastEntryTermIndex().getIndex();
+ final LogEntryProto lastEntry = raftLog.get(lastIndex);
+ Assert.assertTrue(lastEntry.hasMetadataEntry());
+
+ final long loggedCommitIndex = lastEntry.getMetadataEntry().getCommitIndex();
+ final LogEntryProto lastCommittedEntry = raftLog.get(loggedCommitIndex);
+ Assert.assertTrue(lastCommittedEntry.hasStateMachineLogEntry());
+
+ final SimpleStateMachine4Testing leaderStateMachine = SimpleStateMachine4Testing.get(server);
+ final TermIndex lastAppliedTermIndex = leaderStateMachine.getLastAppliedTermIndex();
+ Assert.assertEquals(lastCommittedEntry.getTerm(), lastAppliedTermIndex.getTerm());
+ Assert.assertTrue(lastCommittedEntry.getIndex() <= lastAppliedTermIndex.getIndex());
+ }
}
diff --git a/ratis-server/src/test/java/org/apache/ratis/server/impl/RaftReconfigurationBaseTest.java b/ratis-server/src/test/java/org/apache/ratis/server/impl/RaftReconfigurationBaseTest.java
index 39f39e7..a8d4ade 100644
--- a/ratis-server/src/test/java/org/apache/ratis/server/impl/RaftReconfigurationBaseTest.java
+++ b/ratis-server/src/test/java/org/apache/ratis/server/impl/RaftReconfigurationBaseTest.java
@@ -546,7 +546,7 @@ public abstract class RaftReconfigurationBaseTest<CLUSTER extends MiniRaftCluste
// the old leader should have truncated the setConf from the log
JavaUtils.attempt(() -> log.getLastCommittedIndex() >= confIndex,
- 10, 500L, "COMMIT", LOG);
+ 10, ONE_SECOND, "COMMIT", LOG);
Assert.assertTrue(log.get(confIndex).hasConfigurationEntry());
log2 = null;
} finally {
diff --git a/ratis-server/src/test/java/org/apache/ratis/statemachine/RaftSnapshotBaseTest.java b/ratis-server/src/test/java/org/apache/ratis/statemachine/RaftSnapshotBaseTest.java
index f734ae6..8a8401c 100644
--- a/ratis-server/src/test/java/org/apache/ratis/statemachine/RaftSnapshotBaseTest.java
+++ b/ratis-server/src/test/java/org/apache/ratis/statemachine/RaftSnapshotBaseTest.java
@@ -183,7 +183,7 @@ public abstract class RaftSnapshotBaseTest extends BaseTest {
LOG.info("nextIndex = {}", nextIndex);
final List<File> snapshotFiles = getSnapshotFiles(cluster, nextIndex - SNAPSHOT_TRIGGER_THRESHOLD, nextIndex);
JavaUtils.attempt(() -> snapshotFiles.stream().anyMatch(RaftSnapshotBaseTest::exists),
- 10, 1000, "snapshotFile.exist", LOG);
+ 10, ONE_SECOND, "snapshotFile.exist", LOG);
logs = storageDirectory.getLogSegmentFiles();
} finally {
cluster.shutdown();
diff --git a/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftLogIndex.java b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftLogIndex.java
new file mode 100644
index 0000000..8636b5a
--- /dev/null
+++ b/ratis-test/src/test/java/org/apache/ratis/server/storage/TestRaftLogIndex.java
@@ -0,0 +1,84 @@
+/*
+ * 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.server.storage;
+
+import org.apache.ratis.BaseTest;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.function.BiFunction;
+import java.util.function.Consumer;
+import java.util.function.LongUnaryOperator;
+
+public class TestRaftLogIndex extends BaseTest {
+
+ static void assertUpdate(RaftLogIndex index, BiFunction<RaftLogIndex, Long, Boolean> update,
+ long oldValue, long newValue, boolean expectUpdate) {
+ assertUpdate(index, (i, op) -> update.apply(i, newValue), oldValue, old -> newValue, expectUpdate);
+ }
+
+ static void assertUpdate(RaftLogIndex index, BiFunction<RaftLogIndex, LongUnaryOperator, Boolean> update,
+ long oldValue, LongUnaryOperator op, boolean expectUpdate) {
+ Assert.assertEquals(oldValue, index.get());
+ final boolean updated = update.apply(index, op);
+ Assert.assertEquals(expectUpdate, updated);
+ Assert.assertEquals(expectUpdate? op.applyAsLong(oldValue): oldValue, index.get());
+ }
+
+
+ @Test
+ public void testIndex() {
+ final int initialValue = 900;
+ final RaftLogIndex index = new RaftLogIndex("index", initialValue);
+ Assert.assertEquals(initialValue, index.get());
+
+ final Consumer<Object> log = System.out::println;
+ { // test updateIncreasingly
+ final BiFunction<RaftLogIndex, Long, Boolean> f = (j, n) -> j.updateIncreasingly(n, log);
+ final long i = index.get() + 1;
+ assertUpdate(index, f, i - 1, i, true);
+ assertUpdate(index, f, i, i, false);
+ testFailureCase("updateIncreasingly to a smaller value",
+ () -> assertUpdate(index, f, i, i - 1, false), IllegalStateException.class);
+ }
+
+ { // test updateToMax
+ final BiFunction<RaftLogIndex, Long, Boolean> f = (j, n) -> j.updateToMax(n, log);
+ final long i = index.get() + 1;
+ assertUpdate(index, f, i - 1, i, true);
+ assertUpdate(index, f, i, i, false);
+ assertUpdate(index, f, i, i - 1, false);
+ }
+
+ { // test setUnconditionally
+ final BiFunction<RaftLogIndex, Long, Boolean> f = (j, n) -> j.setUnconditionally(n, log);
+ final long i = index.get() + 1;
+ assertUpdate(index, f, i - 1, i, true);
+ assertUpdate(index, f, i, i, false);
+ assertUpdate(index, f, i, i - 1, true);
+ }
+
+ { // test updateUnconditionally
+ final BiFunction<RaftLogIndex, LongUnaryOperator, Boolean> f = (j, op) -> j.updateUnconditionally(op, log);
+ final long i = index.get() + 1;
+ assertUpdate(index, f, i - 1, n -> n + 1, true);
+ assertUpdate(index, f, i, n -> n, false);
+ assertUpdate(index, f, i, n -> n - 1, true);
+ }
+ }
+}