Simplify anti-entropy code

Change-Id: I6568b1cc7c67e12c5a81ec9f8680f6461813ddce

Simplify anti-entropy code
Change-Id: I6568b1cc7c67e12c5a81ec9f8680f6461813ddce
Jonathan Hart · Gerrit Code Review
Commit 403ea930f2fe912a0c9f8541f88afa012f188bfe 403ea930 1 parent 4d4996e1
Showing 6 changed files with 43 additions and 29 deletions
core/api/src/main/java/org/onosproject/store/Timestamp.java
core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
core/store/dist/src/main/java/org/onosproject/store/ecmap/EventuallyConsistentMapImpl.java
core/store/dist/src/main/java/org/onosproject/store/host/impl/GossipHostStore.java
core/store/dist/src/main/java/org/onosproject/store/impl/Timestamped.java
core/store/dist/src/main/java/org/onosproject/store/link/impl/GossipLinkStore.java
--- a/core/api/src/main/java/org/onosproject/store/Timestamp.java
View file @403ea93
+++ b/core/api/src/main/java/org/onosproject/store/Timestamp.java
View file @403ea93
@@ -15,6 +15,8 @@
  */
 package org.onosproject.store;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Opaque version structure.
  * <p>
@@ -28,4 +30,14 @@ public interface Timestamp extends Comparable<Timestamp> {
     @Override
     public abstract boolean equals(Object obj);
+
+    /**
+     * Tests if this timestamp is newer than the specified timestamp.
+     *
+     * @param other timestamp to compare against
+     * @return true if this instance is newer
+     */
+    public default boolean isNewerThan(Timestamp other) {
+        return this.compareTo(checkNotNull(other)) > 0;
+    }
 }
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
View file @403ea93
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
View file @403ea93
@@ -1170,7 +1170,8 @@ public class GossipDeviceStore
                     Timestamped<DeviceDescription> lProvDevice = lDeviceDescs.getDeviceDesc();
                     Timestamp advDevTimestamp = devAds.get(devFragId);
-                    if (advDevTimestamp == null || lProvDevice.isNewer(advDevTimestamp)) {
+                    if (advDevTimestamp == null || lProvDevice.isNewerThan(
+                            advDevTimestamp)) {
                         // remote does not have it or outdated, suggest
                         notifyPeer(sender, new InternalDeviceEvent(provId, deviceId, lProvDevice));
                     } else if (!lProvDevice.timestamp().equals(advDevTimestamp)) {
@@ -1188,7 +1189,8 @@ public class GossipDeviceStore
                         final PortFragmentId portFragId = new PortFragmentId(deviceId, provId, num);
                         Timestamp advPortTimestamp = portAds.get(portFragId);
-                        if (advPortTimestamp == null || lPort.isNewer(advPortTimestamp)) {
+                        if (advPortTimestamp == null || lPort.isNewerThan(
+                                advPortTimestamp)) {
                             // remote does not have it or outdated, suggest
                             notifyPeer(sender, new InternalPortStatusEvent(provId, deviceId, lPort));
                         } else if (!lPort.timestamp().equals(advPortTimestamp)) {
--- a/core/store/dist/src/main/java/org/onosproject/store/ecmap/EventuallyConsistentMapImpl.java
View file @403ea93
+++ b/core/store/dist/src/main/java/org/onosproject/store/ecmap/EventuallyConsistentMapImpl.java
View file @403ea93
@@ -262,7 +262,7 @@ public class EventuallyConsistentMapImpl<K, V>
     private boolean putInternal(K key, V value, Timestamp timestamp) {
         Timestamp removed = removedItems.get(key);
-        if (removed != null && removed.compareTo(timestamp) > 0) {
+        if (removed != null && removed.isNewerThan(timestamp)) {
             log.debug("ecmap - removed was newer {}", value);
             return false;
         }
@@ -270,7 +270,7 @@ public class EventuallyConsistentMapImpl<K, V>
         boolean success;
         synchronized (this) {
             Timestamped<V> existing = items.get(key);
-            if (existing != null && existing.isNewer(timestamp)) {
+            if (existing != null && existing.isNewerThan(timestamp)) {
                 log.debug("ecmap - existing was newer {}", value);
                 success = false;
             } else {
@@ -305,7 +305,7 @@ public class EventuallyConsistentMapImpl<K, V>
     private boolean removeInternal(K key, Timestamp timestamp) {
         Timestamped<V> value = items.get(key);
         if (value != null) {
-            if (value.isNewer(timestamp)) {
+            if (value.isNewerThan(timestamp)) {
                 return false;
             } else {
                 items.remove(key, value);
@@ -315,7 +315,7 @@ public class EventuallyConsistentMapImpl<K, V>
         Timestamp removedTimestamp = removedItems.get(key);
         if (removedTimestamp == null) {
             return removedItems.putIfAbsent(key, timestamp) == null;
-        } else if (timestamp.compareTo(removedTimestamp) > 0) {
+        } else if (timestamp.isNewerThan(removedTimestamp)) {
             return removedItems.replace(key, removedTimestamp, timestamp);
         } else {
             return false;
@@ -614,7 +614,7 @@ public class EventuallyConsistentMapImpl<K, V>
                 remoteTimestamp = ad.tombstones().get(key);
             }
             if (remoteTimestamp == null || localValue
-                    .isNewer(remoteTimestamp)) {
+                    .isNewerThan(remoteTimestamp)) {
                 // local value is more recent, push to sender
                 updatesToSend
                         .add(new PutEntry<>(key, localValue.value(),
@@ -623,7 +623,7 @@ public class EventuallyConsistentMapImpl<K, V>
             Timestamp remoteDeadTimestamp = ad.tombstones().get(key);
             if (remoteDeadTimestamp != null &&
-                    remoteDeadTimestamp.compareTo(localValue.timestamp()) > 0) {
+                    remoteDeadTimestamp.isNewerThan(localValue.timestamp())) {
                 // sender has a more recent remove
                 if (removeInternal(key, remoteDeadTimestamp)) {
                     externalEvents.add(new EventuallyConsistentMapEvent<>(
@@ -664,7 +664,7 @@ public class EventuallyConsistentMapImpl<K, V>
             Timestamp remoteLiveTimestamp = ad.timestamps().get(key);
             if (remoteLiveTimestamp != null
-                    && localDeadTimestamp.compareTo(remoteLiveTimestamp) > 0) {
+                    && localDeadTimestamp.isNewerThan(remoteLiveTimestamp)) {
                 // sender has zombie, push remove
                 removesToSend
                         .add(new RemoveEntry<>(key, localDeadTimestamp));
@@ -702,18 +702,19 @@ public class EventuallyConsistentMapImpl<K, V>
             Timestamped<V> local = items.get(key);
             Timestamp localDead = removedItems.get(key);
-            if (local != null
+            if (local != null && remoteDeadTimestamp.isNewerThan(
-                    && remoteDeadTimestamp.compareTo(local.timestamp()) > 0) {
+                    local.timestamp())) {
-                // remove our version
+                // If the remote has a more recent tombstone than either our local
+                // value, then do a remove with their timestamp
                 if (removeInternal(key, remoteDeadTimestamp)) {
                     externalEvents.add(new EventuallyConsistentMapEvent<>(
                             EventuallyConsistentMapEvent.Type.REMOVE, key, null));
                 }
-            } else if (localDead != null &&
+            } else if (localDead != null && remoteDeadTimestamp.isNewerThan(
-                    remoteDeadTimestamp.compareTo(localDead) > 0) {
+                    localDead)) {
-                // If we both had the item as removed, but their timestamp is
+                // If the remote has a more recent tombstone than us, update ours
-                // newer, update ours to the newer value
+                // to their timestamp
-                removedItems.put(key, remoteDeadTimestamp);
+                removeInternal(key, remoteDeadTimestamp);
             }
         }
--- a/core/store/dist/src/main/java/org/onosproject/store/host/impl/GossipHostStore.java
View file @403ea93
+++ b/core/store/dist/src/main/java/org/onosproject/store/host/impl/GossipHostStore.java
View file @403ea93
@@ -235,7 +235,7 @@ public class GossipHostStore
     private boolean isHostRemoved(HostId hostId, Timestamp timestamp) {
         Timestamped<Host> removedInfo = removedHosts.get(hostId);
         if (removedInfo != null) {
-            if (removedInfo.isNewer(timestamp)) {
+            if (removedInfo.isNewerThan(timestamp)) {
                 return true;
             }
             removedHosts.remove(hostId, removedInfo);
--- a/core/store/dist/src/main/java/org/onosproject/store/impl/Timestamped.java
View file @403ea93
+++ b/core/store/dist/src/main/java/org/onosproject/store/impl/Timestamped.java
View file @403ea93
@@ -15,13 +15,12 @@
  */
 package org.onosproject.store.impl;
-import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.common.base.MoreObjects;
+import org.onosproject.store.Timestamp;
 import java.util.Objects;
-import org.onosproject.store.Timestamp;
+import static com.google.common.base.Preconditions.checkNotNull;
-
-import com.google.common.base.MoreObjects;
 /**
  * Wrapper class to store Timestamped value.
@@ -69,17 +68,17 @@ public final class Timestamped<T> {
      * @return true if this instance is newer.
      */
     public boolean isNewer(Timestamped<T> other) {
-        return isNewer(checkNotNull(other).timestamp());
+        return isNewerThan(checkNotNull(other).timestamp());
     }
     /**
      * Tests if this timestamp is newer than the specified timestamp.
+     *
      * @param other timestamp to compare against
      * @return true if this instance is newer
      */
-    //TODO put this in Timestamp
+    public boolean isNewerThan(Timestamp other) {
-    public boolean isNewer(Timestamp other) {
+        return timestamp.isNewerThan(other);
-        return this.timestamp.compareTo(checkNotNull(other)) > 0;
     }
     @Override
--- a/core/store/dist/src/main/java/org/onosproject/store/link/impl/GossipLinkStore.java
View file @403ea93
+++ b/core/store/dist/src/main/java/org/onosproject/store/link/impl/GossipLinkStore.java
View file @403ea93
@@ -360,7 +360,7 @@ public class GossipLinkStore
             // only if this request is more recent.
             Timestamp linkRemovedTimestamp = removedLinks.get(key);
             if (linkRemovedTimestamp != null) {
-                if (linkDescription.isNewer(linkRemovedTimestamp)) {
+                if (linkDescription.isNewerThan(linkRemovedTimestamp)) {
                     removedLinks.remove(key);
                 } else {
                     log.trace("Link {} was already removed ignoring.", key);
@@ -762,7 +762,7 @@ public class GossipLinkStore
                         remoteTimestamp = ad.linkTombstones().get(key);
                     }
                     if (remoteTimestamp == null ||
-                        pDesc.isNewer(remoteTimestamp)) {
+                        pDesc.isNewerThan(remoteTimestamp)) {
                         // I have more recent link description. update peer.
                         notifyPeer(sender, new InternalLinkEvent(providerId, pDesc));
                     } else {
@@ -776,7 +776,7 @@ public class GossipLinkStore
                     // search local latest along the way
                     if (localLatest == null ||
-                        pDesc.isNewer(localLatest)) {
+                        pDesc.isNewerThan(localLatest)) {
                         localLatest = pDesc.timestamp();
                     }
                 }