Separate ShardRecoveryCoordinator implementations

This patch turns ShardRecoveryCoordinator into two concrete
implementations based on presence of snapshot. This will allow
us to catch cases where restore from snapshot gives us a null
snapshot.

Furthermore it cleans up the test a bit, byu sharing coordination
instantiation code and using assertTrue/assertFalse to prevent
boxing operations implied by assertEquals.

Change-Id: I8e4ceba0ec6df70e5557f6f457c60ed2cd39a8bd
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
index 1c85aef97899bc464f103d217089a6b9da137eb1..1789f09d2b2681b53220f70114fd7b6825cf2c2e 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
@@ -839,8 +839,11 @@ public class Shard extends RaftActor {
     @Override
     @Nonnull
     protected RaftActorRecoveryCohort getRaftActorRecoveryCohort() {
     @Override
     @Nonnull
     protected RaftActorRecoveryCohort getRaftActorRecoveryCohort() {

-        return new ShardRecoveryCoordinator(store,
-            restoreFromSnapshot != null ? restoreFromSnapshot.getSnapshot() : null, persistenceId(), LOG);
+        if (restoreFromSnapshot == null) {
+            return ShardRecoveryCoordinator.create(store, persistenceId(), LOG);
+        }
+
+        return ShardRecoveryCoordinator.forSnapshot(store, persistenceId(), LOG, restoreFromSnapshot.getSnapshot());

     }
 
     @Override
     }
 
     @Override

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinator.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinator.java
index 6e5886132a26d098262f9ee7a073130bb38962b6..5fc3c7adc262242a261aa0cb145a1ec5c9005e7e 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinator.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinator.java
@@ -27,21 +27,51 @@ import org.slf4j.Logger;
  *
  * @author Thomas Pantelis
  */
  *
  * @author Thomas Pantelis
  */

-class ShardRecoveryCoordinator implements RaftActorRecoveryCohort {
+abstract class ShardRecoveryCoordinator implements RaftActorRecoveryCohort {
+    private static final class Simple extends ShardRecoveryCoordinator {
+        Simple(final ShardDataTree store, final String shardName, final Logger log) {
+            super(store, shardName, log);
+        }
+
+        @Override
+        public Snapshot getRestoreFromSnapshot() {
+            return null;
+        }
+    }
+
+    private static final class WithSnapshot extends ShardRecoveryCoordinator {
+        private final Snapshot restoreFromSnapshot;
+
+        WithSnapshot(final ShardDataTree store, final String shardName, final Logger log, final Snapshot snapshot) {
+            super(store, shardName, log);
+            this.restoreFromSnapshot = Preconditions.checkNotNull(snapshot);
+        }
+
+        @Override
+        public Snapshot getRestoreFromSnapshot() {
+            return restoreFromSnapshot;
+        }
+    }
+

     private final ShardDataTree store;
     private final String shardName;
     private final Logger log;
     private final ShardDataTree store;
     private final String shardName;
     private final Logger log;

-    private final Snapshot restoreFromSnapshot;

 
     private boolean open;
 
 
     private boolean open;
 

-    ShardRecoveryCoordinator(final ShardDataTree store,  final Snapshot restoreFromSnapshot, final String shardName,
-            final Logger log) {
+    ShardRecoveryCoordinator(final ShardDataTree store, final String shardName, final Logger log) {

         this.store = Preconditions.checkNotNull(store);
         this.shardName = Preconditions.checkNotNull(shardName);
         this.log = Preconditions.checkNotNull(log);
         this.store = Preconditions.checkNotNull(store);
         this.shardName = Preconditions.checkNotNull(shardName);
         this.log = Preconditions.checkNotNull(log);

+    }
+
+    static ShardRecoveryCoordinator create(final ShardDataTree store, final String shardName, final Logger log) {
+        return new Simple(store, shardName, log);
+    }

 
 

-        this.restoreFromSnapshot = restoreFromSnapshot;
+    static ShardRecoveryCoordinator forSnapshot(final ShardDataTree store, final String shardName, final Logger log,
+            final Snapshot snapshot) {
+        return new WithSnapshot(store, shardName, log, snapshot);

     }
 
     @Override
     }
 
     @Override


@@ -104,9 +134,4 @@ class ShardRecoveryCoordinator implements RaftActorRecoveryCohort {
                     shardName, shardSnapshot, f), e);
         }
     }
                     shardName, shardSnapshot, f), e);
         }
     }

-
-    @Override
-    public Snapshot getRestoreFromSnapshot() {
-        return restoreFromSnapshot;
-    }

 }
 }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinatorTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinatorTest.java
index fc257b8cbbdf3fa8fd67295b4924dfa7b5c11ac9..4467817a212fa752a557a36c1cf8ec91a2aecfd5 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinatorTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinatorTest.java
@@ -8,7 +8,8 @@
 
 package org.opendaylight.controller.cluster.datastore;
 
 
 package org.opendaylight.controller.cluster.datastore;
 

-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;

 import static org.junit.Assert.fail;
 
 import com.google.common.base.Optional;
 import static org.junit.Assert.fail;
 
 import com.google.common.base.Optional;


@@ -32,13 +33,16 @@ import org.opendaylight.yangtools.yang.data.api.schema.tree.TreeType;
 import org.opendaylight.yangtools.yang.data.impl.schema.tree.InMemoryDataTreeFactory;
 import org.opendaylight.yangtools.yang.data.impl.schema.tree.SchemaValidationFailedException;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 import org.opendaylight.yangtools.yang.data.impl.schema.tree.InMemoryDataTreeFactory;
 import org.opendaylight.yangtools.yang.data.impl.schema.tree.SchemaValidationFailedException;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;

+import org.slf4j.Logger;

 import org.slf4j.LoggerFactory;
 
 public class ShardRecoveryCoordinatorTest extends AbstractTest {
 import org.slf4j.LoggerFactory;
 
 public class ShardRecoveryCoordinatorTest extends AbstractTest {

+    private static final Logger FOO_LOGGER = LoggerFactory.getLogger("foo");

 
     private ShardDataTree peopleDataTree;
     private SchemaContext peopleSchemaContext;
     private SchemaContext carsSchemaContext;
 
     private ShardDataTree peopleDataTree;
     private SchemaContext peopleSchemaContext;
     private SchemaContext carsSchemaContext;

+    private ShardRecoveryCoordinator coordinator;

 
     @Before
     public void setUp() {
 
     @Before
     public void setUp() {


@@ -48,13 +52,12 @@ public class ShardRecoveryCoordinatorTest extends AbstractTest {
         final Shard mockShard = Mockito.mock(Shard.class);
 
         peopleDataTree = new ShardDataTree(mockShard, peopleSchemaContext, TreeType.OPERATIONAL);
         final Shard mockShard = Mockito.mock(Shard.class);
 
         peopleDataTree = new ShardDataTree(mockShard, peopleSchemaContext, TreeType.OPERATIONAL);

+        coordinator = ShardRecoveryCoordinator.create(peopleDataTree, "foobar", FOO_LOGGER);
+        coordinator.startLogRecoveryBatch(10);

     }
 
     @Test
     public void testAppendRecoveredLogEntryCommitTransactionPayload() throws IOException {
     }
 
     @Test
     public void testAppendRecoveredLogEntryCommitTransactionPayload() throws IOException {

-        final ShardRecoveryCoordinator coordinator = new ShardRecoveryCoordinator(peopleDataTree,
-                null, "foobar", LoggerFactory.getLogger("foo"));
-        coordinator.startLogRecoveryBatch(10);

         try {
             coordinator.appendRecoveredLogEntry(CommitTransactionPayload.create(nextTransactionId(), createCar()));
         } catch (final SchemaValidationFailedException e) {
         try {
             coordinator.appendRecoveredLogEntry(CommitTransactionPayload.create(nextTransactionId(), createCar()));
         } catch (final SchemaValidationFailedException e) {


@@ -66,23 +69,15 @@ public class ShardRecoveryCoordinatorTest extends AbstractTest {
 
     @Test
     public void testApplyRecoverySnapshot() {
 
     @Test
     public void testApplyRecoverySnapshot() {

-        final ShardRecoveryCoordinator coordinator = new ShardRecoveryCoordinator(peopleDataTree,
-                null, "foobar", LoggerFactory.getLogger("foo"));
-        coordinator.startLogRecoveryBatch(10);
-

         coordinator.applyRecoverySnapshot(createSnapshot());
 
         coordinator.applyRecoverySnapshot(createSnapshot());
 

-        assertEquals(false, readCars(peopleDataTree).isPresent());
-        assertEquals(true, readPeople(peopleDataTree).isPresent());
+        assertFalse(readCars(peopleDataTree).isPresent());
+        assertTrue(readPeople(peopleDataTree).isPresent());

     }
 
 
     @Test
     public void testApplyCurrentLogRecoveryBatch() {
     }
 
 
     @Test
     public void testApplyCurrentLogRecoveryBatch() {

-        final ShardRecoveryCoordinator coordinator = new ShardRecoveryCoordinator(peopleDataTree,
-                null, "foobar", LoggerFactory.getLogger("foo"));
-        coordinator.startLogRecoveryBatch(10);
-

         try {
             coordinator.applyCurrentLogRecoveryBatch();
         } catch (final IllegalArgumentException e) {
         try {
             coordinator.applyCurrentLogRecoveryBatch();
         } catch (final IllegalArgumentException e) {