From a4d01b62cc1e8d220355aa41632b1a425e8d9652 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 2 Dec 2022 08:47:06 +0100 Subject: [PATCH] Remove support for pre-Fluorine versions We are carrying support for talking RAFT all the way to the initial version. This is not necessary and a bit inefficient. This patch removes support for talking to anything older than Fluorine GA. JIRA: CONTROLLER-2082 Change-Id: Ib9b345aed478004f37f334abae2e3a2879e96512 Signed-off-by: Robert Varga --- .../cluster/raft/FollowerLogInformation.java | 15 ++-- .../controller/cluster/raft/RaftVersions.java | 11 ++- .../raft/behaviors/AbstractLeader.java | 10 ++- .../cluster/raft/messages/AppendEntries.java | 69 +------------------ .../raft/messages/AppendEntriesReply.java | 58 +--------------- .../cluster/raft/behaviors/LeaderTest.java | 2 +- .../raft/messages/AppendEntriesReplyTest.java | 20 ------ .../raft/messages/AppendEntriesTest.java | 29 +------- 8 files changed, 29 insertions(+), 185 deletions(-) diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java index a76d6a29c2..f5c94fbf4c 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java @@ -7,6 +7,7 @@ */ package org.opendaylight.controller.cluster.raft; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static java.util.Objects.requireNonNull; @@ -42,11 +43,8 @@ public final class FollowerLogInformation { private short payloadVersion = -1; - // Assume the HELIUM_VERSION version initially for backwards compatibility until we obtain the follower's - // actual version via AppendEntriesReply. Although we no longer support the Helium version, a pre-Boron - // follower will not have the version field in AppendEntriesReply so it will be set to 0 which is - // HELIUM_VERSION. - private short raftVersion = RaftVersions.HELIUM_VERSION; + // Assume the FLUORINE_VERSION version initially, as we no longer support pre-Fluorine versions. + private short raftVersion = RaftVersions.FLUORINE_VERSION; private final PeerInfo peerInfo; @@ -65,7 +63,7 @@ public final class FollowerLogInformation { */ @VisibleForTesting FollowerLogInformation(final PeerInfo peerInfo, final long matchIndex, final RaftActorContext context) { - this.nextIndex = context.getCommitIndex(); + nextIndex = context.getCommitIndex(); this.matchIndex = matchIndex; this.context = context; this.peerInfo = requireNonNull(peerInfo); @@ -299,6 +297,7 @@ public final class FollowerLogInformation { * @param raftVersion the raft version. */ public void setRaftVersion(final short raftVersion) { + checkArgument(raftVersion >= RaftVersions.FLUORINE_VERSION, "Unexpected version %s", raftVersion); this.raftVersion = raftVersion; } @@ -317,8 +316,8 @@ public final class FollowerLogInformation { * @param state the LeaderInstallSnapshotState */ public void setLeaderInstallSnapshotState(final @NonNull LeaderInstallSnapshotState state) { - if (this.installSnapshotState == null) { - this.installSnapshotState = requireNonNull(state); + if (installSnapshotState == null) { + installSnapshotState = requireNonNull(state); } } diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftVersions.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftVersions.java index 1140e7449f..a09a4aa2cb 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftVersions.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftVersions.java @@ -13,17 +13,14 @@ package org.opendaylight.controller.cluster.raft; * @author Thomas Pantelis */ public final class RaftVersions { - @Deprecated(since = "7.0.0", forRemoval = true) - public static final short HELIUM_VERSION = 0; - @Deprecated(since = "7.0.0", forRemoval = true) - public static final short LITHIUM_VERSION = 1; - @Deprecated(since = "7.0.0", forRemoval = true) - public static final short BORON_VERSION = 3; + // HELIUM_VERSION = 0 + // LITHIUM_VERSION = 1 + // BORON_VERSION = 3 public static final short FLUORINE_VERSION = 4; public static final short ARGON_VERSION = 5; public static final short CURRENT_VERSION = ARGON_VERSION; private RaftVersions() { - + // Hidden on purpose } } diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java index 96c88b541c..c11dbaac92 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java @@ -39,6 +39,7 @@ import org.opendaylight.controller.cluster.raft.FollowerLogInformation; import org.opendaylight.controller.cluster.raft.PeerInfo; import org.opendaylight.controller.cluster.raft.RaftActorContext; import org.opendaylight.controller.cluster.raft.RaftState; +import org.opendaylight.controller.cluster.raft.RaftVersions; import org.opendaylight.controller.cluster.raft.ReplicatedLogEntry; import org.opendaylight.controller.cluster.raft.VotingState; import org.opendaylight.controller.cluster.raft.base.messages.ApplyState; @@ -221,6 +222,13 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior { return this; } + final var followerRaftVersion = appendEntriesReply.getRaftVersion(); + if (followerRaftVersion < RaftVersions.FLUORINE_VERSION) { + log.warn("{}: handleAppendEntriesReply - ignoring reply from follower {} raft version {}", logName(), + followerId, followerRaftVersion); + return this; + } + final long lastActivityNanos = followerLogInformation.nanosSinceLastActivity(); if (lastActivityNanos > context.getConfigParams().getElectionTimeOutInterval().toNanos()) { log.warn("{} : handleAppendEntriesReply delayed beyond election timeout, " @@ -231,7 +239,7 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior { followerLogInformation.markFollowerActive(); followerLogInformation.setPayloadVersion(appendEntriesReply.getPayloadVersion()); - followerLogInformation.setRaftVersion(appendEntriesReply.getRaftVersion()); + followerLogInformation.setRaftVersion(followerRaftVersion); followerLogInformation.setNeedsLeaderAddress(appendEntriesReply.isNeedsLeaderAddress()); long followerLastLogIndex = appendEntriesReply.getLogLastIndex(); diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntries.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntries.java index 5572b01d30..7d7dc9d6fb 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntries.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntries.java @@ -141,16 +141,14 @@ public final class AppendEntries extends AbstractRaftRPC { @Override Object writeReplace() { - if (recipientRaftVersion <= RaftVersions.BORON_VERSION) { - return new Proxy(this); - } - return recipientRaftVersion == RaftVersions.FLUORINE_VERSION ? new ProxyV2(this) : new AE(this); + return recipientRaftVersion <= RaftVersions.FLUORINE_VERSION ? new ProxyV2(this) : new AE(this); } /** * Fluorine version that adds the leader address. */ private static class ProxyV2 implements Externalizable { + @java.io.Serial private static final long serialVersionUID = 1L; private AppendEntries appendEntries; @@ -210,68 +208,7 @@ public final class AppendEntries extends AbstractRaftRPC { leaderAddress); } - private Object readResolve() { - return appendEntries; - } - } - - /** - * Pre-Fluorine version. - */ - @Deprecated - private static class Proxy implements Externalizable { - private static final long serialVersionUID = 1L; - - private AppendEntries appendEntries; - - // checkstyle flags the public modifier as redundant which really doesn't make sense since it clearly isn't - // redundant. It is explicitly needed for Java serialization to be able to create instances via reflection. - @SuppressWarnings("checkstyle:RedundantModifier") - public Proxy() { - } - - Proxy(final AppendEntries appendEntries) { - this.appendEntries = appendEntries; - } - - @Override - public void writeExternal(final ObjectOutput out) throws IOException { - out.writeLong(appendEntries.getTerm()); - out.writeObject(appendEntries.leaderId); - out.writeLong(appendEntries.prevLogTerm); - out.writeLong(appendEntries.prevLogIndex); - out.writeLong(appendEntries.leaderCommit); - out.writeLong(appendEntries.replicatedToAllIndex); - out.writeShort(appendEntries.payloadVersion); - - out.writeInt(appendEntries.entries.size()); - for (ReplicatedLogEntry e: appendEntries.entries) { - out.writeLong(e.getIndex()); - out.writeLong(e.getTerm()); - out.writeObject(e.getData()); - } - } - - @Override - public void readExternal(final ObjectInput in) throws IOException, ClassNotFoundException { - long term = in.readLong(); - String leaderId = (String) in.readObject(); - long prevLogTerm = in.readLong(); - long prevLogIndex = in.readLong(); - long leaderCommit = in.readLong(); - long replicatedToAllIndex = in.readLong(); - short payloadVersion = in.readShort(); - - int size = in.readInt(); - var entries = ImmutableList.builderWithExpectedSize(size); - for (int i = 0; i < size; i++) { - entries.add(new SimpleReplicatedLogEntry(in.readLong(), in.readLong(), (Payload) in.readObject())); - } - - appendEntries = new AppendEntries(term, leaderId, prevLogIndex, prevLogTerm, entries.build(), leaderCommit, - replicatedToAllIndex, payloadVersion, RaftVersions.CURRENT_VERSION, RaftVersions.BORON_VERSION, null); - } - + @java.io.Serial private Object readResolve() { return appendEntries; } diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesReply.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesReply.java index 7cfe8b1c6d..033a19a7b2 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesReply.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesReply.java @@ -118,16 +118,14 @@ public final class AppendEntriesReply extends AbstractRaftRPC { @Override Object writeReplace() { - if (recipientRaftVersion <= RaftVersions.BORON_VERSION) { - return new Proxy(this); - } - return recipientRaftVersion == RaftVersions.FLUORINE_VERSION ? new Proxy2(this) : new AR(this); + return recipientRaftVersion <= RaftVersions.FLUORINE_VERSION ? new Proxy2(this) : new AR(this); } /** * Fluorine version that adds the needsLeaderAddress flag. */ private static class Proxy2 implements Externalizable { + @java.io.Serial private static final long serialVersionUID = 1L; private AppendEntriesReply appendEntriesReply; @@ -172,57 +170,7 @@ public final class AppendEntriesReply extends AbstractRaftRPC { RaftVersions.CURRENT_VERSION); } - private Object readResolve() { - return appendEntriesReply; - } - } - - /** - * Pre-Fluorine version. - */ - @Deprecated - private static class Proxy implements Externalizable { - private static final long serialVersionUID = 1L; - - private AppendEntriesReply appendEntriesReply; - - // checkstyle flags the public modifier as redundant which really doesn't make sense since it clearly isn't - // redundant. It is explicitly needed for Java serialization to be able to create instances via reflection. - @SuppressWarnings("checkstyle:RedundantModifier") - public Proxy() { - } - - Proxy(final AppendEntriesReply appendEntriesReply) { - this.appendEntriesReply = appendEntriesReply; - } - - @Override - public void writeExternal(final ObjectOutput out) throws IOException { - out.writeShort(appendEntriesReply.raftVersion); - out.writeLong(appendEntriesReply.getTerm()); - out.writeObject(appendEntriesReply.followerId); - out.writeBoolean(appendEntriesReply.success); - out.writeLong(appendEntriesReply.logLastIndex); - out.writeLong(appendEntriesReply.logLastTerm); - out.writeShort(appendEntriesReply.payloadVersion); - out.writeBoolean(appendEntriesReply.forceInstallSnapshot); - } - - @Override - public void readExternal(final ObjectInput in) throws IOException, ClassNotFoundException { - short raftVersion = in.readShort(); - long term = in.readLong(); - String followerId = (String) in.readObject(); - boolean success = in.readBoolean(); - long logLastIndex = in.readLong(); - long logLastTerm = in.readLong(); - short payloadVersion = in.readShort(); - boolean forceInstallSnapshot = in.readBoolean(); - - appendEntriesReply = new AppendEntriesReply(followerId, term, success, logLastIndex, logLastTerm, - payloadVersion, forceInstallSnapshot, false, raftVersion, RaftVersions.CURRENT_VERSION); - } - + @java.io.Serial private Object readResolve() { return appendEntriesReply; } diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java index d1990213f8..c3f3b3296b 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java +++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java @@ -1728,7 +1728,7 @@ public class LeaderTest extends AbstractLeaderTest { FollowerLogInformation followerInfo = leader.getFollower(FOLLOWER_ID); assertEquals(payloadVersion, leader.getLeaderPayloadVersion()); - assertEquals(RaftVersions.HELIUM_VERSION, followerInfo.getRaftVersion()); + assertEquals(RaftVersions.FLUORINE_VERSION, followerInfo.getRaftVersion()); AppendEntriesReply reply = new AppendEntriesReply(FOLLOWER_ID, 1, true, 2, 1, payloadVersion); diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesReplyTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesReplyTest.java index 8b2c5ccf89..79c7477ba2 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesReplyTest.java +++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesReplyTest.java @@ -37,24 +37,4 @@ public class AppendEntriesReplyTest { assertEquals("isForceInstallSnapshot", expected.isForceInstallSnapshot(), cloned.isForceInstallSnapshot()); assertEquals("isNeedsLeaderAddress", expected.isNeedsLeaderAddress(), cloned.isNeedsLeaderAddress()); } - - @Test - @Deprecated - public void testPreFluorineSerialization() { - final var expected = new AppendEntriesReply("follower", 5, true, 100, 4, (short)6, true, true, - RaftVersions.BORON_VERSION); - - final var bytes = SerializationUtils.serialize(expected); - assertEquals(141, bytes.length); - final var cloned = (AppendEntriesReply) SerializationUtils.deserialize(bytes); - - assertEquals("getTerm", expected.getTerm(), cloned.getTerm()); - assertEquals("getFollowerId", expected.getFollowerId(), cloned.getFollowerId()); - assertEquals("getLogLastTerm", expected.getLogLastTerm(), cloned.getLogLastTerm()); - assertEquals("getLogLastIndex", expected.getLogLastIndex(), cloned.getLogLastIndex()); - assertEquals("getPayloadVersion", expected.getPayloadVersion(), cloned.getPayloadVersion()); - assertEquals("getRaftVersion", expected.getRaftVersion(), cloned.getRaftVersion()); - assertEquals("isForceInstallSnapshot", expected.isForceInstallSnapshot(), cloned.isForceInstallSnapshot()); - assertEquals("isNeedsLeaderAddress", false, cloned.isNeedsLeaderAddress()); - } } diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesTest.java index 91eb5d096e..3e3b6184b9 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesTest.java +++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/messages/AppendEntriesTest.java @@ -8,7 +8,6 @@ package org.opendaylight.controller.cluster.raft.messages; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import java.util.Iterator; import java.util.List; @@ -56,25 +55,6 @@ public class AppendEntriesTest { verifyAppendEntries(expected, cloned, RaftVersions.CURRENT_VERSION); } - @Test - @Deprecated - public void testPreFluorineSerialization() { - ReplicatedLogEntry entry1 = new SimpleReplicatedLogEntry(1, 2, new MockPayload("payload1")); - - ReplicatedLogEntry entry2 = new SimpleReplicatedLogEntry(3, 4, new MockPayload("payload2")); - - short payloadVersion = 5; - - final var expected = new AppendEntries(5L, "node1", 7L, 8L, List.of(entry1, entry2), 10L, -1, - payloadVersion, RaftVersions.BORON_VERSION, "leader address"); - - final var bytes = SerializationUtils.serialize(expected); - assertEquals(350, bytes.length); - final var cloned = (AppendEntries) SerializationUtils.deserialize(bytes); - - verifyAppendEntries(expected, cloned, RaftVersions.BORON_VERSION); - } - private static void verifyAppendEntries(final AppendEntries expected, final AppendEntries actual, final short recipientRaftVersion) { assertEquals("getLeaderId", expected.getLeaderId(), actual.getLeaderId()); @@ -91,13 +71,8 @@ public class AppendEntriesTest { verifyReplicatedLogEntry(iter.next(), e); } - if (recipientRaftVersion > RaftVersions.BORON_VERSION) { - assertEquals("getLeaderAddress", expected.getLeaderAddress(), actual.getLeaderAddress()); - assertEquals("getLeaderRaftVersion", RaftVersions.CURRENT_VERSION, actual.getLeaderRaftVersion()); - } else { - assertFalse(actual.getLeaderAddress().isPresent()); - assertEquals("getLeaderRaftVersion", RaftVersions.BORON_VERSION, actual.getLeaderRaftVersion()); - } + assertEquals("getLeaderAddress", expected.getLeaderAddress(), actual.getLeaderAddress()); + assertEquals("getLeaderRaftVersion", RaftVersions.CURRENT_VERSION, actual.getLeaderRaftVersion()); } private static void verifyReplicatedLogEntry(final ReplicatedLogEntry expected, final ReplicatedLogEntry actual) { -- 2.36.6