Do not allow persistence callbacks to throw Exception

Since we are indirecting through an executor, we are forced
to wrap any exception -- just do not bother, as the callbacks
are executed in the context of an actor anyway (dealing with
RuntimeExceptions) and users are not throwing checked exceptions.

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

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ReplicatedLog.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ReplicatedLog.java
index a05dbc662fd6506af1ed84f31c29cf99c0fbcd84..1a0b38f778f3a5d07f01147e4ce8b3836ee197fd 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ReplicatedLog.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ReplicatedLog.java
@@ -7,8 +7,8 @@
  */
 package org.opendaylight.controller.cluster.raft;
 
-import akka.japi.Procedure;
 import java.util.List;
+import java.util.function.Consumer;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 
@@ -88,7 +88,7 @@ public interface ReplicatedLog {
      * Appends an entry to the in-memory log and persists it as well.
      *
      * @param replicatedLogEntry the entry to append
-     * @param callback the Procedure to be notified when persistence is complete (optional).
+     * @param callback the callback to be notified when persistence is complete (optional).
      * @param doAsync if true, the persistent actor can receive subsequent messages to process in between the persist
      *        call and the execution of the associated callback. If false, subsequent messages are stashed and get
      *        delivered after persistence is complete and the associated callback is executed. In either case the
@@ -96,7 +96,7 @@ public interface ReplicatedLog {
      * @return true if the entry was successfully appended, false otherwise.
      */
     boolean appendAndPersist(@NonNull ReplicatedLogEntry replicatedLogEntry,
-            @Nullable Procedure<ReplicatedLogEntry> callback, boolean doAsync);
+            @Nullable Consumer<ReplicatedLogEntry> callback, boolean doAsync);
 
     /**
      * Returns a list of log entries starting from the given index to the end of the log.

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ReplicatedLogImpl.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ReplicatedLogImpl.java
index c53cad68c5def14c0dae3718ffc01144235db5fe..c22f6f431bd4cf833ab6eb7c275563e9d357d386 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ReplicatedLogImpl.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ReplicatedLogImpl.java
@@ -9,9 +9,9 @@ package org.opendaylight.controller.cluster.raft;
 
 import static java.util.Objects.requireNonNull;
 
-import akka.japi.Procedure;
 import java.util.Collections;
 import java.util.List;
+import java.util.function.Consumer;
 import org.opendaylight.controller.cluster.raft.persisted.DeleteEntries;
 import org.opendaylight.controller.cluster.raft.persisted.Snapshot;
 
@@ -93,7 +93,7 @@ final class ReplicatedLogImpl extends AbstractReplicatedLogImpl {
 
     @Override
     public boolean appendAndPersist(final ReplicatedLogEntry replicatedLogEntry,
-            final Procedure<ReplicatedLogEntry> callback, final boolean doAsync)  {
+            final Consumer<ReplicatedLogEntry> callback, final boolean doAsync)  {
 
         context.getLogger().debug("{}: Append log entry and persist {} ", context.getId(), replicatedLogEntry);
 
@@ -112,24 +112,18 @@ final class ReplicatedLogImpl extends AbstractReplicatedLogImpl {
     }
 
     private void persistCallback(final ReplicatedLogEntry persistedLogEntry,
-            final Procedure<ReplicatedLogEntry> callback) {
+            final Consumer<ReplicatedLogEntry> callback) {
         context.getExecutor().execute(() -> syncPersistCallback(persistedLogEntry, callback));
     }
 
-    @SuppressWarnings("checkstyle:illegalCatch")
     private void syncPersistCallback(final ReplicatedLogEntry persistedLogEntry,
-            final Procedure<ReplicatedLogEntry> callback) {
+            final Consumer<ReplicatedLogEntry> callback) {
         context.getLogger().debug("{}: persist complete {}", context.getId(), persistedLogEntry);
 
         dataSizeSinceLastSnapshot += persistedLogEntry.size();
 
         if (callback != null) {
-            try {
-                callback.apply(persistedLogEntry);
-            } catch (Exception e) {
-                context.getLogger().error("{}: persist callback failed", context.getId(), e);
-                throw new IllegalStateException("Persist callback failed", e);
-            }
+            callback.accept(persistedLogEntry);
         }
     }
 }

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
index 4fa4dbfb57b34f620eecd6b82710dffc754cd88f..b193a549f76f4e6a033da3b8d39bc4c4d5c1f929 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
@@ -14,7 +14,6 @@ import akka.cluster.Cluster;
 import akka.cluster.ClusterEvent.CurrentClusterState;
 import akka.cluster.Member;
 import akka.cluster.MemberStatus;
-import akka.japi.Procedure;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Stopwatch;
 import java.io.IOException;
@@ -24,6 +23,7 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Consumer;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.controller.cluster.messaging.MessageAssembler;
 import org.opendaylight.controller.cluster.raft.RaftActorContext;
@@ -303,7 +303,7 @@ public class Follower extends AbstractRaftActorBehavior {
         // applied to the state already, as the persistence callback occurs async, and we want those entries
         // purged from the persisted log as well.
         final AtomicBoolean shouldCaptureSnapshot = new AtomicBoolean(false);
-        final Procedure<ReplicatedLogEntry> appendAndPersistCallback = logEntry -> {
+        final Consumer<ReplicatedLogEntry> appendAndPersistCallback = logEntry -> {
             final List<ReplicatedLogEntry> entries = appendEntries.getEntries();
             final ReplicatedLogEntry lastEntryToAppend = entries.get(entries.size() - 1);
             if (shouldCaptureSnapshot.get() && logEntry == lastEntryToAppend) {

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/AbstractReplicatedLogImplTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/AbstractReplicatedLogImplTest.java
index 8ff68310243ac8129fb719ed9150cad1c5c69123..5b72cb3117a378cd59c9960079e19417d57ed08d 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/AbstractReplicatedLogImplTest.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/AbstractReplicatedLogImplTest.java
@@ -13,10 +13,10 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
-import akka.japi.Procedure;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Consumer;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -325,8 +325,11 @@ public class AbstractReplicatedLogImplTest {
         }
 
         @Override
-        public boolean appendAndPersist(ReplicatedLogEntry replicatedLogEntry, Procedure<ReplicatedLogEntry> callback,
+        public boolean appendAndPersist(ReplicatedLogEntry replicatedLogEntry, Consumer<ReplicatedLogEntry> callback,
                 boolean doAsync) {
+            if (callback != null) {
+                callback.accept(replicatedLogEntry);
+            }
             return true;
         }
 

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/MockRaftActorContext.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/MockRaftActorContext.java
index 1935e2650398e74cd4ea33e071e2387b975ec7bd..8c17e1e8e8e9d4060221f8c68b2a980f3f6b1b32 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/MockRaftActorContext.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/MockRaftActorContext.java
@@ -12,7 +12,6 @@ import akka.actor.ActorRef;
 import akka.actor.ActorSelection;
 import akka.actor.ActorSystem;
 import akka.actor.Props;
-import akka.japi.Procedure;
 import com.google.common.io.ByteSource;
 import com.google.common.util.concurrent.MoreExecutors;
 import java.io.IOException;
@@ -190,17 +189,11 @@ public class MockRaftActorContext extends RaftActorContextImpl {
         @Override
         @SuppressWarnings("checkstyle:IllegalCatch")
         public boolean appendAndPersist(final ReplicatedLogEntry replicatedLogEntry,
-                final Procedure<ReplicatedLogEntry> callback, final boolean doAsync) {
+                final Consumer<ReplicatedLogEntry> callback, final boolean doAsync) {
             append(replicatedLogEntry);
 
             if (callback != null) {
-                try {
-                    callback.apply(replicatedLogEntry);
-                } catch (RuntimeException e) {
-                    throw e;
-                } catch (Exception e) {
-                    throw new RuntimeException(e);
-                }
+                callback.accept(replicatedLogEntry);
             }
 
             return true;

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/ReplicatedLogImplTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/ReplicatedLogImplTest.java
index 276ace15b533eb8d653e128268aeabee56a9ee9b..8e9c6c9271f70d8a2becdcdb89f8cb236a3cdb3b 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/ReplicatedLogImplTest.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/ReplicatedLogImplTest.java
@@ -19,6 +19,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions;
 import akka.japi.Procedure;
 import com.google.common.util.concurrent.MoreExecutors;
 import java.util.Collections;
+import java.util.function.Consumer;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -92,12 +93,12 @@ public class ReplicatedLogImplTest {
         reset(mockPersistence);
 
         ReplicatedLogEntry logEntry2 = new SimpleReplicatedLogEntry(2, 1, new MockPayload("2"));
-        Procedure<ReplicatedLogEntry> mockCallback = mock(Procedure.class);
+        Consumer<ReplicatedLogEntry> mockCallback = mock(Consumer.class);
         log.appendAndPersist(logEntry2, mockCallback, true);
 
         verifyPersist(logEntry2);
 
-        verify(mockCallback).apply(same(logEntry2));
+        verify(mockCallback).accept(same(logEntry2));
 
         assertEquals("size", 2, log.size());
     }
@@ -107,7 +108,7 @@ public class ReplicatedLogImplTest {
     public void testAppendAndPersisWithDuplicateEntry() throws Exception {
         ReplicatedLog log = ReplicatedLogImpl.newInstance(context);
 
-        Procedure<ReplicatedLogEntry> mockCallback = mock(Procedure.class);
+        Consumer<ReplicatedLogEntry> mockCallback = mock(Consumer.class);
         ReplicatedLogEntry logEntry = new SimpleReplicatedLogEntry(1, 1, new MockPayload("1"));
 
         log.appendAndPersist(logEntry, mockCallback, true);