From: Tomas Cere <tomas.cere@pantheon.tech>
Date: Mon, 3 Jun 2019 13:02:08 +0000 (+0200)
Subject: Fix multiple snapshots present after journal removal
X-Git-Tag: release/sodium~39
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=609076e66a0298d0d3c912ade66e813a464c4c8a

Fix multiple snapshots present after journal removal

After journal removal the sequence number starts counting from 0
once again. Therefore we can have multiple snapshots present and
snapshots with higher sequence number always take priority for loading
even when a newer snapshot is present.

Change this up in 2 ways:
1. disregard sequence number while deleting snapshots(delete all
snapshots with an older timestamp)
2. while loading snapshots prioritize timestamp rather than seqNr.

Also simplify LocalSnapshotStore.compare().

Change-Id: I205ea0ddf48d73b0a09297a1ce4e9fd514531993
Signed-off-by: Tomas Cere <tomas.cere@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/SnapshotManager.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/SnapshotManager.java
index 8674ef4f2b..02269a289b 100644
--- 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
@@ -469,7 +469,7 @@ public class SnapshotManager implements SnapshotState {
                 context.getReplicatedLog().snapshotCommit();
             }
 
-            context.getPersistenceProvider().deleteSnapshots(new SnapshotSelectionCriteria(sequenceNumber,
+            context.getPersistenceProvider().deleteSnapshots(new SnapshotSelectionCriteria(scala.Long.MaxValue(),
                     timeStamp - 1, 0L, 0L));
 
             context.getPersistenceProvider().deleteMessages(lastSequenceNumber);
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 0dc33130da..14b94d9fb7 100644
--- 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
@@ -448,7 +448,7 @@ public class SnapshotManagerTest extends AbstractActorTest {
 
         verify(mockDataPersistenceProvider).deleteSnapshots(criteriaCaptor.capture());
 
-        assertEquals(100L, criteriaCaptor.getValue().maxSequenceNr());
+        assertEquals(Long.MAX_VALUE, criteriaCaptor.getValue().maxSequenceNr());
         assertEquals(1233L, criteriaCaptor.getValue().maxTimestamp());
 
         MessageCollectorActor.expectFirstMatching(actorRef, SnapshotComplete.class);
diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStore.java b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStore.java
index e08fec2e89..42c90b80d4 100644
--- a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStore.java
+++ b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStore.java
@@ -7,6 +7,8 @@
  */
 package org.opendaylight.controller.cluster.persistence;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import akka.actor.ExtendedActorSystem;
 import akka.dispatch.Futures;
 import akka.persistence.SelectedSnapshot;
@@ -342,9 +344,9 @@ public class LocalSnapshotStore extends SnapshotStore {
 
     @VisibleForTesting
     static int compare(final SnapshotMetadata m1, final SnapshotMetadata m2) {
-        return (int) (!m1.persistenceId().equals(m2.persistenceId())
-                ? m1.persistenceId().compareTo(m2.persistenceId()) :
-            m1.sequenceNr() != m2.sequenceNr() ? m1.sequenceNr() - m2.sequenceNr() :
-                m1.timestamp() != m2.timestamp() ? m1.timestamp() - m2.timestamp() : 0);
+        checkArgument(m1.persistenceId().equals(m2.persistenceId()),
+                "Persistence id does not match. id1: %s, id2: %s", m1.persistenceId(), m2.persistenceId());
+        final int cmp = Long.compare(m1.timestamp(), m2.timestamp());
+        return cmp != 0 ? cmp : Long.compare(m1.sequenceNr(), m2.sequenceNr());
     }
 }