Do not reset datasize after a fake snapshot

If we reset dataSize from SnapshotManager's fake snapshot, we would
not have correctly accounted for on-disk size.

Fix this by exposing a simple knob, which allows suppressing dataSize
update during snapshotCommit().

JIRA: CONTROLLER-1957
Change-Id: I553ddb523ac8504892cc5353bfe4b002c25436ce
Signed-off-by: tadei.bilan <tadei.bilan@pantheon.tech>
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/AbstractReplicatedLogImpl.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/AbstractReplicatedLogImpl.java
index 42af1502ee00ce3a5fe76d47e3be22c1bb769d40..64506ee6867fedd656b190f420be7ac7ec44c9b9 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/AbstractReplicatedLogImpl.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/AbstractReplicatedLogImpl.java
@@ -256,18 +256,20 @@ public abstract class AbstractReplicatedLogImpl implements ReplicatedLog {
     }
 
     @Override
     }
 
     @Override

-    public void snapshotCommit() {
+    public void snapshotCommit(final boolean updateDataSize) {

         snapshottedJournal = null;
         previousSnapshotIndex = -1;
         previousSnapshotTerm = -1;
 
         snapshottedJournal = null;
         previousSnapshotIndex = -1;
         previousSnapshotTerm = -1;
 

-        // need to recalc the datasize based on the entries left after precommit.
-        int newDataSize = 0;
-        for (ReplicatedLogEntry logEntry : journal) {
-            newDataSize += logEntry.size();
+        if (updateDataSize) {
+            // need to recalc the datasize based on the entries left after precommit.
+            int newDataSize = 0;
+            for (ReplicatedLogEntry logEntry : journal) {
+                newDataSize += logEntry.size();
+            }
+            LOG.trace("{}: Updated dataSize from {} to {}", logContext, dataSize, newDataSize);
+            dataSize = newDataSize;

         }
         }

-        LOG.trace("{}: Updated dataSize from {} to {}", logContext, dataSize, newDataSize);
-        dataSize = newDataSize;

     }
 
     @Override
     }
 
     @Override

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 1a0b38f778f3a5d07f01147e4ce8b3836ee197fd..8cf133c2ab73ba2c62a9b177d3ea26d802e06abe 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
@@ -186,9 +186,20 @@ public interface ReplicatedLog {
     void snapshotPreCommit(long snapshotCapturedIndex, long snapshotCapturedTerm);
 
     /**
     void snapshotPreCommit(long snapshotCapturedIndex, long snapshotCapturedTerm);
 
     /**

-     * Sets the Replicated log to state after snapshot success.
+     * Sets the Replicated log to state after snapshot success. This method is equivalent to
+     * {@code snapshotCommit(true)}.

      */
      */

-    void snapshotCommit();
+    default void snapshotCommit() {
+        snapshotCommit(true);
+    }
+
+    /**
+     * Sets the Replicated log to state after snapshot success. Most users will want to use {@link #snapshotCommit()}
+     * instead.
+     *
+     * @param updateDataSize true if {@link #dataSize()} should also be updated
+     */
+    void snapshotCommit(boolean updateDataSize);

 
     /**
      * Restores the replicated log to a state in the event of a save snapshot failure.
 
     /**
      * Restores the replicated log to a state in the event of a save snapshot failure.

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/SnapshotManager.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/SnapshotManager.java
index 791a791027ab2a2c2e7dc8786d167514c754dae6..8037fb8d73ce88e6309f6a0f56b6d4fb787e8761 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/SnapshotManager.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/SnapshotManager.java
@@ -265,7 +265,7 @@ public class SnapshotManager implements SnapshotState {
                 //use the term of the temp-min, since we check for isPresent, entry will not be null
                 ReplicatedLogEntry entry = context.getReplicatedLog().get(tempMin);
                 context.getReplicatedLog().snapshotPreCommit(tempMin, entry.getTerm());
                 //use the term of the temp-min, since we check for isPresent, entry will not be null
                 ReplicatedLogEntry entry = context.getReplicatedLog().get(tempMin);
                 context.getReplicatedLog().snapshotPreCommit(tempMin, entry.getTerm());

-                context.getReplicatedLog().snapshotCommit();
+                context.getReplicatedLog().snapshotCommit(false);

                 return tempMin;
             }
 
                 return tempMin;
             }
 

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 8e9c6c9271f70d8a2becdcdb89f8cb236a3cdb3b..35ecd77d390aa41d477aacdad438bb7a62fcee9e 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
@@ -57,16 +57,17 @@ public class ReplicatedLogImplTest {
         MockitoAnnotations.initMocks(this);
 
         context = new RaftActorContextImpl(null, null, "test",
         MockitoAnnotations.initMocks(this);
 
         context = new RaftActorContextImpl(null, null, "test",

-                new ElectionTermImpl(mockPersistence, "test", LOG), -1, -1, Collections.<String,String>emptyMap(),
+                new ElectionTermImpl(mockPersistence, "test", LOG), -1, -1, Collections.emptyMap(),

                 configParams, mockPersistence, applyState -> { }, LOG,  MoreExecutors.directExecutor());
     }
 
                 configParams, mockPersistence, applyState -> { }, LOG,  MoreExecutors.directExecutor());
     }
 

-    private void verifyPersist(Object message) throws Exception {
+    private void verifyPersist(final Object message) throws Exception {

         verifyPersist(message, new Same(message), true);
     }
 
     @SuppressWarnings({ "unchecked", "rawtypes" })
         verifyPersist(message, new Same(message), true);
     }
 
     @SuppressWarnings({ "unchecked", "rawtypes" })

-    private void verifyPersist(Object message, ArgumentMatcher<?> matcher, boolean async) throws Exception {
+    private void verifyPersist(final Object message, final ArgumentMatcher<?> matcher, final boolean async)
+            throws Exception {

         ArgumentCaptor<Procedure> procedure = ArgumentCaptor.forClass(Procedure.class);
         if (async) {
             verify(mockPersistence).persistAsync(argThat(matcher), procedure.capture());
         ArgumentCaptor<Procedure> procedure = ArgumentCaptor.forClass(Procedure.class);
         if (async) {
             verify(mockPersistence).persistAsync(argThat(matcher), procedure.capture());


@@ -196,7 +197,21 @@ public class ReplicatedLogImplTest {
         verifyNoMoreInteractions(mockPersistence);
     }
 
         verifyNoMoreInteractions(mockPersistence);
     }
 

-    public ArgumentMatcher<DeleteEntries> match(final DeleteEntries actual) {
+    @Test
+    public void testCommitFakeSnapshot() {
+        ReplicatedLog log = ReplicatedLogImpl.newInstance(context);
+
+        log.append(new SimpleReplicatedLogEntry(0, 1, new MockPayload("0")));
+        final int dataSizeAfterFirstPayload = log.dataSize();
+
+        log.snapshotPreCommit(0,1);
+        log.snapshotCommit(false);
+
+        assertEquals(0, log.size());
+        assertEquals(dataSizeAfterFirstPayload, log.dataSize());
+    }
+
+    private static ArgumentMatcher<DeleteEntries> match(final DeleteEntries actual) {

         return other -> actual.getFromIndex() == other.getFromIndex();
     }
 }
         return other -> actual.getFromIndex() == other.getFromIndex();
     }
 }

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/SnapshotManagerTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/SnapshotManagerTest.java
index 88e9a613caa576ce1a803f7b2141c7f2041c0c9f..4cdec8ca672c87fb091f176185e18837c52e0a4f 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/SnapshotManagerTest.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/SnapshotManagerTest.java
@@ -560,7 +560,7 @@ public class SnapshotManagerTest extends AbstractActorTest {
         assertEquals("return index", 10L, retIndex);
 
         verify(mockReplicatedLog).snapshotPreCommit(10, 5);
         assertEquals("return index", 10L, retIndex);
 
         verify(mockReplicatedLog).snapshotPreCommit(10, 5);

-        verify(mockReplicatedLog).snapshotCommit();
+        verify(mockReplicatedLog).snapshotCommit(false);

 
         verify(mockRaftActorBehavior, never()).setReplicatedToAllIndex(anyLong());
     }
 
         verify(mockRaftActorBehavior, never()).setReplicatedToAllIndex(anyLong());
     }


@@ -578,7 +578,7 @@ public class SnapshotManagerTest extends AbstractActorTest {
         assertEquals("return index", -1L, retIndex);
 
         verify(mockReplicatedLog, never()).snapshotPreCommit(anyLong(), anyLong());
         assertEquals("return index", -1L, retIndex);
 
         verify(mockReplicatedLog, never()).snapshotPreCommit(anyLong(), anyLong());

-        verify(mockReplicatedLog, never()).snapshotCommit();
+        verify(mockReplicatedLog, never()).snapshotCommit(false);

 
         verify(mockRaftActorBehavior, never()).setReplicatedToAllIndex(anyLong());
     }
 
         verify(mockRaftActorBehavior, never()).setReplicatedToAllIndex(anyLong());
     }


@@ -596,7 +596,7 @@ public class SnapshotManagerTest extends AbstractActorTest {
         assertEquals("return index", -1L, retIndex);
 
         verify(mockReplicatedLog, never()).snapshotPreCommit(anyLong(), anyLong());
         assertEquals("return index", -1L, retIndex);
 
         verify(mockReplicatedLog, never()).snapshotPreCommit(anyLong(), anyLong());

-        verify(mockReplicatedLog, never()).snapshotCommit();
+        verify(mockReplicatedLog, never()).snapshotCommit(false);

 
         verify(mockRaftActorBehavior, never()).setReplicatedToAllIndex(anyLong());
     }
 
         verify(mockRaftActorBehavior, never()).setReplicatedToAllIndex(anyLong());
     }


@@ -611,7 +611,7 @@ public class SnapshotManagerTest extends AbstractActorTest {
         assertEquals("return index", -1L, retIndex);
 
         verify(mockReplicatedLog, never()).snapshotPreCommit(anyLong(), anyLong());
         assertEquals("return index", -1L, retIndex);
 
         verify(mockReplicatedLog, never()).snapshotPreCommit(anyLong(), anyLong());

-        verify(mockReplicatedLog, never()).snapshotCommit();
+        verify(mockReplicatedLog, never()).snapshotCommit(false);

 
         // Trim index is greater than replicatedToAllIndex so should update it.
         verify(mockRaftActorBehavior).setReplicatedToAllIndex(10L);
 
         // Trim index is greater than replicatedToAllIndex so should update it.
         verify(mockRaftActorBehavior).setReplicatedToAllIndex(10L);


@@ -635,8 +635,7 @@ public class SnapshotManagerTest extends AbstractActorTest {
         snapshotManager.trimLog(10);
 
         verify(mockReplicatedLog, never()).snapshotPreCommit(anyLong(), anyLong());
         snapshotManager.trimLog(10);
 
         verify(mockReplicatedLog, never()).snapshotPreCommit(anyLong(), anyLong());

-        verify(mockReplicatedLog, never()).snapshotCommit();
-
+        verify(mockReplicatedLog, never()).snapshotCommit(false);

     }
 
     @Test
     }
 
     @Test


@@ -658,7 +657,6 @@ public class SnapshotManagerTest extends AbstractActorTest {
 
         verify(mockReplicatedLog, never()).snapshotPreCommit(10, 5);
         verify(mockReplicatedLog, never()).snapshotCommit();
 
         verify(mockReplicatedLog, never()).snapshotPreCommit(10, 5);
         verify(mockReplicatedLog, never()).snapshotCommit();

-

     }
 
     @Test
     }
 
     @Test