Separate ShardRecoveryCoordinator implementations 60/49260/7
authorRobert Varga <robert.varga@pantheon.tech>
Thu, 31 Aug 2017 20:48:04 +0000 (22:48 +0200)
committerTom Pantelis <tompantelis@gmail.com>
Fri, 1 Sep 2017 14:07:57 +0000 (14:07 +0000)
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>
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinator.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardRecoveryCoordinatorTest.java

index 1c85aef..1789f09 100644 (file)
@@ -839,8 +839,11 @@ public class Shard extends RaftActor {
     @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
index 6e58861..5fc3c7a 100644 (file)
@@ -27,21 +27,51 @@ import org.slf4j.Logger;
  *
  * @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 Snapshot restoreFromSnapshot;
 
     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);
+    }
+
+    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
@@ -104,9 +134,4 @@ class ShardRecoveryCoordinator implements RaftActorRecoveryCohort {
                     shardName, shardSnapshot, f), e);
         }
     }
-
-    @Override
-    public Snapshot getRestoreFromSnapshot() {
-        return restoreFromSnapshot;
-    }
 }
index fc257b8..4467817 100644 (file)
@@ -8,7 +8,8 @@
 
 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;
@@ -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.slf4j.Logger;
 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 ShardRecoveryCoordinator coordinator;
 
     @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);
+        coordinator = ShardRecoveryCoordinator.create(peopleDataTree, "foobar", FOO_LOGGER);
+        coordinator.startLogRecoveryBatch(10);
     }
 
     @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) {
@@ -66,23 +69,15 @@ public class ShardRecoveryCoordinatorTest extends AbstractTest {
 
     @Test
     public void testApplyRecoverySnapshot() {
-        final ShardRecoveryCoordinator coordinator = new ShardRecoveryCoordinator(peopleDataTree,
-                null, "foobar", LoggerFactory.getLogger("foo"));
-        coordinator.startLogRecoveryBatch(10);
-
         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() {
-        final ShardRecoveryCoordinator coordinator = new ShardRecoveryCoordinator(peopleDataTree,
-                null, "foobar", LoggerFactory.getLogger("foo"));
-        coordinator.startLogRecoveryBatch(10);
-
         try {
             coordinator.applyCurrentLogRecoveryBatch();
         } catch (final IllegalArgumentException e) {

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.