ONOS-3565: Retry host store updates that fail due to concurrent modification
Change-Id: Id2af2795b0c9f9b1c8d0c4985781ff24e576c7e3
diff --git a/core/api/src/main/java/org/onosproject/store/service/ConsistentMapException.java b/core/api/src/main/java/org/onosproject/store/service/ConsistentMapException.java
index 94ed649..20280327 100644
--- a/core/api/src/main/java/org/onosproject/store/service/ConsistentMapException.java
+++ b/core/api/src/main/java/org/onosproject/store/service/ConsistentMapException.java
@@ -24,6 +24,10 @@
public ConsistentMapException() {
}
+ public ConsistentMapException(String message) {
+ super(message);
+ }
+
public ConsistentMapException(Throwable t) {
super(t);
}
@@ -38,6 +42,13 @@
* ConsistentMap update conflicts with an in flight transaction.
*/
public static class ConcurrentModification extends ConsistentMapException {
+ public ConcurrentModification() {
+ super();
+ }
+
+ public ConcurrentModification(String message) {
+ super(message);
+ }
}
/**
diff --git a/core/api/src/main/java/org/onosproject/store/service/StorageException.java b/core/api/src/main/java/org/onosproject/store/service/StorageException.java
index a66fc3e..b55d2c2 100644
--- a/core/api/src/main/java/org/onosproject/store/service/StorageException.java
+++ b/core/api/src/main/java/org/onosproject/store/service/StorageException.java
@@ -24,6 +24,10 @@
public StorageException() {
}
+ public StorageException(String message) {
+ super(message);
+ }
+
public StorageException(Throwable t) {
super(t);
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/consistent/impl/DefaultAsyncConsistentMap.java b/core/store/dist/src/main/java/org/onosproject/store/consistent/impl/DefaultAsyncConsistentMap.java
index af2bb74..7c2613e 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/consistent/impl/DefaultAsyncConsistentMap.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/consistent/impl/DefaultAsyncConsistentMap.java
@@ -27,6 +27,7 @@
import org.onosproject.core.ApplicationId;
import org.onosproject.store.service.AsyncConsistentMap;
import org.onosproject.store.service.ConsistentMapException;
+import org.onosproject.store.service.ConsistentMapException.ConcurrentModification;
import org.onosproject.store.service.MapEvent;
import org.onosproject.store.service.MapEventListener;
import org.onosproject.store.service.Serializer;
@@ -293,7 +294,7 @@
if (v.updated()) {
return v.newValue();
} else {
- throw new ConsistentMapException.ConcurrentModification();
+ throw new ConcurrentModification("Concurrent update to " + name + " detected");
}
});
});
diff --git a/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java b/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java
index a2e8a09..c76cb7f 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
+
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -27,6 +28,7 @@
import org.onlab.packet.MacAddress;
import org.onlab.packet.VlanId;
import org.onlab.util.KryoNamespace;
+import org.onlab.util.Tools;
import org.onosproject.net.Annotations;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.DefaultAnnotations;
@@ -43,10 +45,12 @@
import org.onosproject.store.AbstractStore;
import org.onosproject.store.serializers.KryoNamespaces;
import org.onosproject.store.service.ConsistentMap;
+import org.onosproject.store.service.ConsistentMapException;
import org.onosproject.store.service.MapEvent;
import org.onosproject.store.service.MapEventListener;
import org.onosproject.store.service.Serializer;
import org.onosproject.store.service.StorageService;
+import org.onosproject.store.service.Versioned;
import org.slf4j.Logger;
import java.util.Collection;
@@ -56,6 +60,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
+import java.util.function.Supplier;
import java.util.stream.Collectors;
import static com.google.common.base.Preconditions.checkNotNull;
@@ -155,37 +160,42 @@
HostId hostId,
HostDescription hostDescription,
boolean replaceIPs) {
- // TODO: We need a way to detect conflicting changes and abort update.
- host.computeIf(hostId,
+ Supplier<Versioned<DefaultHost>> supplier =
+ () -> host.computeIf(hostId,
existingHost -> shouldUpdate(existingHost, providerId, hostId,
hostDescription, replaceIPs),
(id, existingHost) -> {
- HostLocation location = hostDescription.location();
+ HostLocation location = hostDescription.location();
- final Set<IpAddress> addresses;
- if (existingHost == null || replaceIPs) {
- addresses = ImmutableSet.copyOf(hostDescription.ipAddress());
- } else {
- addresses = Sets.newHashSet(existingHost.ipAddresses());
- addresses.addAll(hostDescription.ipAddress());
- }
+ final Set<IpAddress> addresses;
+ if (existingHost == null || replaceIPs) {
+ addresses = ImmutableSet.copyOf(hostDescription.ipAddress());
+ } else {
+ addresses = Sets.newHashSet(existingHost.ipAddresses());
+ addresses.addAll(hostDescription.ipAddress());
+ }
- final Annotations annotations;
- if (existingHost != null) {
- annotations = merge((DefaultAnnotations) existingHost.annotations(),
- hostDescription.annotations());
- } else {
- annotations = hostDescription.annotations();
- }
+ final Annotations annotations;
+ if (existingHost != null) {
+ annotations = merge((DefaultAnnotations) existingHost.annotations(),
+ hostDescription.annotations());
+ } else {
+ annotations = hostDescription.annotations();
+ }
- return new DefaultHost(providerId,
- hostId,
- hostDescription.hwAddress(),
- hostDescription.vlan(),
- location,
- addresses,
- annotations);
- });
+ return new DefaultHost(providerId,
+ hostId,
+ hostDescription.hwAddress(),
+ hostDescription.vlan(),
+ location,
+ addresses,
+ annotations);
+ });
+
+ Tools.retryable(supplier,
+ ConsistentMapException.ConcurrentModification.class,
+ Integer.MAX_VALUE,
+ 50).get();
return null;
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java b/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java
index 0335ba5..f15bc86 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java
@@ -86,6 +86,7 @@
.build();
childMap.put(ResourcePath.ROOT, ImmutableList.of());
+ log.info("Started");
}
@Override