From 93ccb43ad0f5c78337f19884a51e2bd479cc46fd Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Thu, 29 Oct 2015 10:27:28 -0400 Subject: [PATCH] Always persist ServerConfigurationPayload log entries We need to always persist ServerConfigurationPayload log entries regardless of whether or not persistence is enabled for the derived RaftActor's data. I added a new tagging interface PersistentPayload, implemented by ServerConfigurationPayload, to indicate a Payload needs to always be persisted. Since log entries are persisted by both the RaftActor and Follower behavior via the ReplicatedLog, the logic to determine persistence based on PersistentPayload needs to be available to both. The ReplicatedLog uses the persistence provider contained in the RaftActorContext which is the DelegatingPersistentDataProvider set by the RaftActor. So to keep the rest of the code the same and keep it simple, I derived a RaftActorDelegatingPersistentDataProvider which overrides persist to handle the PersistentPayload logic utilizing the RaftActor's existing PersistentDataProvider. Change-Id: I243026b28ed57461ad92324b6947091ae74a7127 Signed-off-by: Tom Pantelis --- .../controller/cluster/raft/RaftActor.java | 4 +- ...ActorDelegatingPersistentDataProvider.java | 49 ++++++++ .../raft/ServerConfigurationPayload.java | 3 +- ...rDelegatingPersistentDataProviderTest.java | 113 ++++++++++++++++++ ...ftActorServerConfigurationSupportTest.java | 16 +++ .../client/messages/PersistentPayload.java | 17 +++ 6 files changed, 200 insertions(+), 2 deletions(-) create mode 100644 opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorDelegatingPersistentDataProvider.java create mode 100644 opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorDelegatingPersistentDataProviderTest.java create mode 100644 opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/raft/protobuff/client/messages/PersistentPayload.java diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java index a043c69cf7..4eb9ad8359 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java @@ -108,7 +108,7 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { */ private final RaftActorContextImpl context; - private final DelegatingPersistentDataProvider delegatingPersistenceProvider = new DelegatingPersistentDataProvider(null); + private final DelegatingPersistentDataProvider delegatingPersistenceProvider; private final PersistentDataProvider persistentProvider; @@ -126,6 +126,8 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor { Optional configParams, short payloadVersion) { persistentProvider = new PersistentDataProvider(this); + delegatingPersistenceProvider = new RaftActorDelegatingPersistentDataProvider(null, persistentProvider); + context = new RaftActorContextImpl(this.getSelf(), this.getContext(), id, new ElectionTermImpl(persistentProvider, id, LOG), -1, -1, peerAddresses, diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorDelegatingPersistentDataProvider.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorDelegatingPersistentDataProvider.java new file mode 100644 index 0000000000..466609d893 --- /dev/null +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorDelegatingPersistentDataProvider.java @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2015 Brocade Communications Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.controller.cluster.raft; + +import akka.japi.Procedure; +import com.google.common.base.Preconditions; +import org.opendaylight.controller.cluster.DataPersistenceProvider; +import org.opendaylight.controller.cluster.DelegatingPersistentDataProvider; +import org.opendaylight.controller.cluster.PersistentDataProvider; +import org.opendaylight.controller.cluster.raft.protobuff.client.messages.PersistentPayload; + +/** + * The DelegatingPersistentDataProvider used by RaftActor to override the configured persistent provider to + * persist ReplicatedLogEntry's based on whether or not the payload is a PersistentPayload instance. + * + * @author Thomas Pantelis + */ +class RaftActorDelegatingPersistentDataProvider extends DelegatingPersistentDataProvider { + private final PersistentDataProvider persistentProvider; + + RaftActorDelegatingPersistentDataProvider(DataPersistenceProvider delegate, + PersistentDataProvider persistentProvider) { + super(delegate); + this.persistentProvider = Preconditions.checkNotNull(persistentProvider); + } + + @Override + public void persist(T o, Procedure procedure) { + if(getDelegate().isRecoveryApplicable()) { + super.persist(o, procedure); + } else { + boolean isPersistentPayload = false; + if(o instanceof ReplicatedLogEntry) { + isPersistentPayload = ((ReplicatedLogEntry)o).getData() instanceof PersistentPayload; + } + + if(isPersistentPayload) { + persistentProvider.persist(o, procedure); + } else { + super.persist(o, procedure); + } + } + } +} diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ServerConfigurationPayload.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ServerConfigurationPayload.java index 112c7d56e0..b439cef035 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ServerConfigurationPayload.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ServerConfigurationPayload.java @@ -17,6 +17,7 @@ import java.util.List; import java.util.Map; import javax.annotation.Nonnull; import org.opendaylight.controller.cluster.raft.protobuff.client.messages.Payload; +import org.opendaylight.controller.cluster.raft.protobuff.client.messages.PersistentPayload; import org.opendaylight.controller.protobuff.messages.cluster.raft.AppendEntriesMessages; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -26,7 +27,7 @@ import org.slf4j.LoggerFactory; * * @author Thomas Pantelis */ -public class ServerConfigurationPayload extends Payload implements Serializable { +public class ServerConfigurationPayload extends Payload implements PersistentPayload, Serializable { private static final long serialVersionUID = 1L; private static final Logger LOG = LoggerFactory.getLogger(ServerConfigurationPayload.class); diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorDelegatingPersistentDataProviderTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorDelegatingPersistentDataProviderTest.java new file mode 100644 index 0000000000..378c53cb18 --- /dev/null +++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorDelegatingPersistentDataProviderTest.java @@ -0,0 +1,113 @@ +/* + * Copyright (c) 2015 Brocade Communications Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.controller.cluster.raft; + +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.verify; +import akka.japi.Procedure; +import com.google.protobuf.GeneratedMessage.GeneratedExtension; +import java.util.Map; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.opendaylight.controller.cluster.DataPersistenceProvider; +import org.opendaylight.controller.cluster.PersistentDataProvider; +import org.opendaylight.controller.cluster.raft.protobuff.client.messages.Payload; +import org.opendaylight.controller.cluster.raft.protobuff.client.messages.PersistentPayload; + +/** + * Unit tests for RaftActorDelegatingPersistentDataProvider. + * + * @author Thomas Pantelis + */ +public class RaftActorDelegatingPersistentDataProviderTest { + private static final Payload PERSISTENT_PAYLOAD = new TestPersistentPayload(); + + private static final Payload NON_PERSISTENT_PAYLOAD = new TestNonPersistentPayload(); + + private static final Object OTHER_DATA_OBJECT = new Object(); + + @Mock + private ReplicatedLogEntry mockPersistentLogEntry; + + @Mock + private ReplicatedLogEntry mockNonPersistentLogEntry; + + @Mock + private DataPersistenceProvider mockDelegateProvider; + + @Mock + private PersistentDataProvider mockPersistentProvider; + + @SuppressWarnings("rawtypes") + @Mock + private Procedure mockProcedure; + + private RaftActorDelegatingPersistentDataProvider provider; + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + doReturn(PERSISTENT_PAYLOAD).when(mockPersistentLogEntry).getData(); + doReturn(NON_PERSISTENT_PAYLOAD).when(mockNonPersistentLogEntry).getData(); + provider = new RaftActorDelegatingPersistentDataProvider(mockDelegateProvider, mockPersistentProvider); + } + + @SuppressWarnings("unchecked") + @Test + public void testPersistWithPersistenceEnabled() { + doReturn(true).when(mockDelegateProvider).isRecoveryApplicable(); + + provider.persist(mockPersistentLogEntry, mockProcedure); + verify(mockDelegateProvider).persist(mockPersistentLogEntry, mockProcedure); + + provider.persist(mockNonPersistentLogEntry, mockProcedure); + verify(mockDelegateProvider).persist(mockNonPersistentLogEntry, mockProcedure); + + provider.persist(OTHER_DATA_OBJECT, mockProcedure); + verify(mockDelegateProvider).persist(OTHER_DATA_OBJECT, mockProcedure); + } + + @SuppressWarnings("unchecked") + @Test + public void testPersistWithPersistenceDisabled() { + doReturn(false).when(mockDelegateProvider).isRecoveryApplicable(); + + provider.persist(mockPersistentLogEntry, mockProcedure); + verify(mockPersistentProvider).persist(mockPersistentLogEntry, mockProcedure); + + provider.persist(mockNonPersistentLogEntry, mockProcedure); + verify(mockDelegateProvider).persist(mockNonPersistentLogEntry, mockProcedure); + + provider.persist(OTHER_DATA_OBJECT, mockProcedure); + verify(mockDelegateProvider).persist(OTHER_DATA_OBJECT, mockProcedure); + } + + static class TestNonPersistentPayload extends Payload { + @SuppressWarnings("rawtypes") + @Override + public Map encode() { + return null; + } + + @Override + public Payload decode( + org.opendaylight.controller.protobuff.messages.cluster.raft.AppendEntriesMessages.AppendEntries.ReplicatedLogEntry.Payload payload) { + return null; + } + + @Override + public int size() { + return 0; + } + } + + static class TestPersistentPayload extends TestNonPersistentPayload implements PersistentPayload { + } +} diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupportTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupportTest.java index 883680b06d..3d6f7f414c 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupportTest.java +++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupportTest.java @@ -178,6 +178,21 @@ public class RaftActorServerConfigurationSupportTest extends AbstractActorTest { assertEquals("Follower last applied index", 3, followerActorContext.getLastApplied()); assertEquals("New follower commit index", 3, newFollowerActorContext.getCommitIndex()); assertEquals("New follower last applied index", 3, newFollowerActorContext.getLastApplied()); + + List persistedLogEntries = InMemoryJournal.get(LEADER_ID, ReplicatedLogImplEntry.class); + assertEquals("Leader ReplicatedLogImplEntry entries", 1, persistedLogEntries.size()); + ReplicatedLogImplEntry logEntry = persistedLogEntries.get(0); + assertEquals("Leader ReplicatedLogImplEntry getTerm", 1, logEntry.getTerm()); + assertEquals("Leader ReplicatedLogImplEntry getIndex", 3, logEntry.getIndex()); + assertEquals("Leader ReplicatedLogImplEntry getData", ServerConfigurationPayload.class, logEntry.getData().getClass()); + + persistedLogEntries = InMemoryJournal.get(NEW_SERVER_ID, ReplicatedLogImplEntry.class); + assertEquals("New follower ReplicatedLogImplEntry entries", 1, persistedLogEntries.size()); + logEntry = persistedLogEntries.get(0); + assertEquals("New follower ReplicatedLogImplEntry getTerm", 1, logEntry.getTerm()); + assertEquals("New follower ReplicatedLogImplEntry getIndex", 3, logEntry.getIndex()); + assertEquals("New follower ReplicatedLogImplEntry getData", ServerConfigurationPayload.class, + logEntry.getData().getClass()); } @Test @@ -701,6 +716,7 @@ public class RaftActorServerConfigurationSupportTest extends AbstractActorTest { public static class MockNewFollowerRaftActor extends AbstractMockRaftActor { public MockNewFollowerRaftActor(ConfigParams config, TestActorRef collectorActor) { super(NEW_SERVER_ID, Maps.newHashMap(), Optional.of(config), null, collectorActor); + setPersistence(false); } static Props props(ConfigParams config, TestActorRef collectorActor) { diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/raft/protobuff/client/messages/PersistentPayload.java b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/raft/protobuff/client/messages/PersistentPayload.java new file mode 100644 index 0000000000..828ed2cbac --- /dev/null +++ b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/raft/protobuff/client/messages/PersistentPayload.java @@ -0,0 +1,17 @@ +/* + * Copyright (c) 2015 Brocade Communications Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.controller.cluster.raft.protobuff.client.messages; + +/** + * This is a tagging interface for a Payload implementation that needs to always be persisted regardless of + * whether or not the component is configured to be persistent. + * + * @author Thomas Pantelis + */ +public interface PersistentPayload { +} -- 2.36.6