Committed by
Gerrit Code Review
ONOS-3565: Retry host store updates that fail due to concurrent modification
Change-Id: Id2af2795b0c9f9b1c8d0c4985781ff24e576c7e3
Showing
5 changed files
with
30 additions
and
3 deletions
... | @@ -24,6 +24,10 @@ public class ConsistentMapException extends StorageException { | ... | @@ -24,6 +24,10 @@ public class ConsistentMapException extends StorageException { |
24 | public ConsistentMapException() { | 24 | public ConsistentMapException() { |
25 | } | 25 | } |
26 | 26 | ||
27 | + public ConsistentMapException(String message) { | ||
28 | + super(message); | ||
29 | + } | ||
30 | + | ||
27 | public ConsistentMapException(Throwable t) { | 31 | public ConsistentMapException(Throwable t) { |
28 | super(t); | 32 | super(t); |
29 | } | 33 | } |
... | @@ -38,6 +42,13 @@ public class ConsistentMapException extends StorageException { | ... | @@ -38,6 +42,13 @@ public class ConsistentMapException extends StorageException { |
38 | * ConsistentMap update conflicts with an in flight transaction. | 42 | * ConsistentMap update conflicts with an in flight transaction. |
39 | */ | 43 | */ |
40 | public static class ConcurrentModification extends ConsistentMapException { | 44 | public static class ConcurrentModification extends ConsistentMapException { |
45 | + public ConcurrentModification() { | ||
46 | + super(); | ||
47 | + } | ||
48 | + | ||
49 | + public ConcurrentModification(String message) { | ||
50 | + super(message); | ||
51 | + } | ||
41 | } | 52 | } |
42 | 53 | ||
43 | /** | 54 | /** | ... | ... |
... | @@ -24,6 +24,10 @@ public class StorageException extends RuntimeException { | ... | @@ -24,6 +24,10 @@ public class StorageException extends RuntimeException { |
24 | public StorageException() { | 24 | public StorageException() { |
25 | } | 25 | } |
26 | 26 | ||
27 | + public StorageException(String message) { | ||
28 | + super(message); | ||
29 | + } | ||
30 | + | ||
27 | public StorageException(Throwable t) { | 31 | public StorageException(Throwable t) { |
28 | super(t); | 32 | super(t); |
29 | } | 33 | } | ... | ... |
... | @@ -27,6 +27,7 @@ import org.onlab.util.Tools; | ... | @@ -27,6 +27,7 @@ import org.onlab.util.Tools; |
27 | import org.onosproject.core.ApplicationId; | 27 | import org.onosproject.core.ApplicationId; |
28 | import org.onosproject.store.service.AsyncConsistentMap; | 28 | import org.onosproject.store.service.AsyncConsistentMap; |
29 | import org.onosproject.store.service.ConsistentMapException; | 29 | import org.onosproject.store.service.ConsistentMapException; |
30 | +import org.onosproject.store.service.ConsistentMapException.ConcurrentModification; | ||
30 | import org.onosproject.store.service.MapEvent; | 31 | import org.onosproject.store.service.MapEvent; |
31 | import org.onosproject.store.service.MapEventListener; | 32 | import org.onosproject.store.service.MapEventListener; |
32 | import org.onosproject.store.service.Serializer; | 33 | import org.onosproject.store.service.Serializer; |
... | @@ -293,7 +294,7 @@ public class DefaultAsyncConsistentMap<K, V> implements AsyncConsistentMap<K, V | ... | @@ -293,7 +294,7 @@ public class DefaultAsyncConsistentMap<K, V> implements AsyncConsistentMap<K, V |
293 | if (v.updated()) { | 294 | if (v.updated()) { |
294 | return v.newValue(); | 295 | return v.newValue(); |
295 | } else { | 296 | } else { |
296 | - throw new ConsistentMapException.ConcurrentModification(); | 297 | + throw new ConcurrentModification("Concurrent update to " + name + " detected"); |
297 | } | 298 | } |
298 | }); | 299 | }); |
299 | }); | 300 | }); | ... | ... |
... | @@ -17,6 +17,7 @@ package org.onosproject.store.host.impl; | ... | @@ -17,6 +17,7 @@ package org.onosproject.store.host.impl; |
17 | 17 | ||
18 | import com.google.common.collect.ImmutableSet; | 18 | import com.google.common.collect.ImmutableSet; |
19 | import com.google.common.collect.Sets; | 19 | import com.google.common.collect.Sets; |
20 | + | ||
20 | import org.apache.felix.scr.annotations.Activate; | 21 | import org.apache.felix.scr.annotations.Activate; |
21 | import org.apache.felix.scr.annotations.Component; | 22 | import org.apache.felix.scr.annotations.Component; |
22 | import org.apache.felix.scr.annotations.Deactivate; | 23 | import org.apache.felix.scr.annotations.Deactivate; |
... | @@ -27,6 +28,7 @@ import org.onlab.packet.IpAddress; | ... | @@ -27,6 +28,7 @@ import org.onlab.packet.IpAddress; |
27 | import org.onlab.packet.MacAddress; | 28 | import org.onlab.packet.MacAddress; |
28 | import org.onlab.packet.VlanId; | 29 | import org.onlab.packet.VlanId; |
29 | import org.onlab.util.KryoNamespace; | 30 | import org.onlab.util.KryoNamespace; |
31 | +import org.onlab.util.Tools; | ||
30 | import org.onosproject.net.Annotations; | 32 | import org.onosproject.net.Annotations; |
31 | import org.onosproject.net.ConnectPoint; | 33 | import org.onosproject.net.ConnectPoint; |
32 | import org.onosproject.net.DefaultAnnotations; | 34 | import org.onosproject.net.DefaultAnnotations; |
... | @@ -43,10 +45,12 @@ import org.onosproject.net.provider.ProviderId; | ... | @@ -43,10 +45,12 @@ import org.onosproject.net.provider.ProviderId; |
43 | import org.onosproject.store.AbstractStore; | 45 | import org.onosproject.store.AbstractStore; |
44 | import org.onosproject.store.serializers.KryoNamespaces; | 46 | import org.onosproject.store.serializers.KryoNamespaces; |
45 | import org.onosproject.store.service.ConsistentMap; | 47 | import org.onosproject.store.service.ConsistentMap; |
48 | +import org.onosproject.store.service.ConsistentMapException; | ||
46 | import org.onosproject.store.service.MapEvent; | 49 | import org.onosproject.store.service.MapEvent; |
47 | import org.onosproject.store.service.MapEventListener; | 50 | import org.onosproject.store.service.MapEventListener; |
48 | import org.onosproject.store.service.Serializer; | 51 | import org.onosproject.store.service.Serializer; |
49 | import org.onosproject.store.service.StorageService; | 52 | import org.onosproject.store.service.StorageService; |
53 | +import org.onosproject.store.service.Versioned; | ||
50 | import org.slf4j.Logger; | 54 | import org.slf4j.Logger; |
51 | 55 | ||
52 | import java.util.Collection; | 56 | import java.util.Collection; |
... | @@ -56,6 +60,7 @@ import java.util.Objects; | ... | @@ -56,6 +60,7 @@ import java.util.Objects; |
56 | import java.util.Set; | 60 | import java.util.Set; |
57 | import java.util.concurrent.ConcurrentHashMap; | 61 | import java.util.concurrent.ConcurrentHashMap; |
58 | import java.util.function.Predicate; | 62 | import java.util.function.Predicate; |
63 | +import java.util.function.Supplier; | ||
59 | import java.util.stream.Collectors; | 64 | import java.util.stream.Collectors; |
60 | 65 | ||
61 | import static com.google.common.base.Preconditions.checkNotNull; | 66 | import static com.google.common.base.Preconditions.checkNotNull; |
... | @@ -155,8 +160,8 @@ public class DistributedHostStore | ... | @@ -155,8 +160,8 @@ public class DistributedHostStore |
155 | HostId hostId, | 160 | HostId hostId, |
156 | HostDescription hostDescription, | 161 | HostDescription hostDescription, |
157 | boolean replaceIPs) { | 162 | boolean replaceIPs) { |
158 | - // TODO: We need a way to detect conflicting changes and abort update. | 163 | + Supplier<Versioned<DefaultHost>> supplier = |
159 | - host.computeIf(hostId, | 164 | + () -> host.computeIf(hostId, |
160 | existingHost -> shouldUpdate(existingHost, providerId, hostId, | 165 | existingHost -> shouldUpdate(existingHost, providerId, hostId, |
161 | hostDescription, replaceIPs), | 166 | hostDescription, replaceIPs), |
162 | (id, existingHost) -> { | 167 | (id, existingHost) -> { |
... | @@ -187,6 +192,11 @@ public class DistributedHostStore | ... | @@ -187,6 +192,11 @@ public class DistributedHostStore |
187 | annotations); | 192 | annotations); |
188 | }); | 193 | }); |
189 | 194 | ||
195 | + Tools.retryable(supplier, | ||
196 | + ConsistentMapException.ConcurrentModification.class, | ||
197 | + Integer.MAX_VALUE, | ||
198 | + 50).get(); | ||
199 | + | ||
190 | return null; | 200 | return null; |
191 | } | 201 | } |
192 | 202 | ... | ... |
... | @@ -86,6 +86,7 @@ public class ConsistentResourceStore extends AbstractStore<ResourceEvent, Resour | ... | @@ -86,6 +86,7 @@ public class ConsistentResourceStore extends AbstractStore<ResourceEvent, Resour |
86 | .build(); | 86 | .build(); |
87 | 87 | ||
88 | childMap.put(ResourcePath.ROOT, ImmutableList.of()); | 88 | childMap.put(ResourcePath.ROOT, ImmutableList.of()); |
89 | + log.info("Started"); | ||
89 | } | 90 | } |
90 | 91 | ||
91 | @Override | 92 | @Override | ... | ... |
-
Please register or login to post a comment