Committed by
Gerrit Code Review
ONOS-4590 - This has been noticed also in ONOS-3633 - Fixing an issue with intents.
Pending intents were not being removed from the Gossip intent store as the intentData object did not match what was stored during the remove. General cause: intent resubmission loops when installation time exceeds cleanup polling interval Change-Id: Id50baade9807c102b7cbff9a4667e048f57698bb
Showing
1 changed file
with
20 additions
and
14 deletions
... | @@ -16,7 +16,6 @@ | ... | @@ -16,7 +16,6 @@ |
16 | package org.onosproject.store.intent.impl; | 16 | package org.onosproject.store.intent.impl; |
17 | 17 | ||
18 | import com.google.common.collect.ImmutableList; | 18 | import com.google.common.collect.ImmutableList; |
19 | - | ||
20 | import org.apache.commons.lang.math.RandomUtils; | 19 | import org.apache.commons.lang.math.RandomUtils; |
21 | import org.apache.felix.scr.annotations.Activate; | 20 | import org.apache.felix.scr.annotations.Activate; |
22 | import org.apache.felix.scr.annotations.Component; | 21 | import org.apache.felix.scr.annotations.Component; |
... | @@ -31,19 +30,19 @@ import org.onosproject.cluster.NodeId; | ... | @@ -31,19 +30,19 @@ import org.onosproject.cluster.NodeId; |
31 | import org.onosproject.net.intent.Intent; | 30 | import org.onosproject.net.intent.Intent; |
32 | import org.onosproject.net.intent.IntentData; | 31 | import org.onosproject.net.intent.IntentData; |
33 | import org.onosproject.net.intent.IntentEvent; | 32 | import org.onosproject.net.intent.IntentEvent; |
33 | +import org.onosproject.net.intent.IntentPartitionService; | ||
34 | import org.onosproject.net.intent.IntentState; | 34 | import org.onosproject.net.intent.IntentState; |
35 | import org.onosproject.net.intent.IntentStore; | 35 | import org.onosproject.net.intent.IntentStore; |
36 | import org.onosproject.net.intent.IntentStoreDelegate; | 36 | import org.onosproject.net.intent.IntentStoreDelegate; |
37 | import org.onosproject.net.intent.Key; | 37 | import org.onosproject.net.intent.Key; |
38 | -import org.onosproject.net.intent.IntentPartitionService; | ||
39 | import org.onosproject.store.AbstractStore; | 38 | import org.onosproject.store.AbstractStore; |
40 | -import org.onosproject.store.service.MultiValuedTimestamp; | ||
41 | -import org.onosproject.store.service.WallClockTimestamp; | ||
42 | import org.onosproject.store.serializers.KryoNamespaces; | 39 | import org.onosproject.store.serializers.KryoNamespaces; |
43 | import org.onosproject.store.service.EventuallyConsistentMap; | 40 | import org.onosproject.store.service.EventuallyConsistentMap; |
44 | import org.onosproject.store.service.EventuallyConsistentMapEvent; | 41 | import org.onosproject.store.service.EventuallyConsistentMapEvent; |
45 | import org.onosproject.store.service.EventuallyConsistentMapListener; | 42 | import org.onosproject.store.service.EventuallyConsistentMapListener; |
43 | +import org.onosproject.store.service.MultiValuedTimestamp; | ||
46 | import org.onosproject.store.service.StorageService; | 44 | import org.onosproject.store.service.StorageService; |
45 | +import org.onosproject.store.service.WallClockTimestamp; | ||
47 | import org.slf4j.Logger; | 46 | import org.slf4j.Logger; |
48 | 47 | ||
49 | import java.util.Collection; | 48 | import java.util.Collection; |
... | @@ -91,7 +90,6 @@ public class GossipIntentStore | ... | @@ -91,7 +90,6 @@ public class GossipIntentStore |
91 | public void activate() { | 90 | public void activate() { |
92 | KryoNamespace.Builder intentSerializer = KryoNamespace.newBuilder() | 91 | KryoNamespace.Builder intentSerializer = KryoNamespace.newBuilder() |
93 | .register(KryoNamespaces.API) | 92 | .register(KryoNamespaces.API) |
94 | - .nextId(KryoNamespaces.BEGIN_USER_CUSTOM_ID) | ||
95 | .register(IntentData.class) | 93 | .register(IntentData.class) |
96 | .register(MultiValuedTimestamp.class); | 94 | .register(MultiValuedTimestamp.class); |
97 | 95 | ||
... | @@ -99,16 +97,17 @@ public class GossipIntentStore | ... | @@ -99,16 +97,17 @@ public class GossipIntentStore |
99 | .withName("intent-current") | 97 | .withName("intent-current") |
100 | .withSerializer(intentSerializer) | 98 | .withSerializer(intentSerializer) |
101 | .withTimestampProvider((key, intentData) -> | 99 | .withTimestampProvider((key, intentData) -> |
102 | - new MultiValuedTimestamp<>(intentData.version(), | 100 | + new MultiValuedTimestamp<>(intentData.version(), |
103 | - sequenceNumber.getAndIncrement())) | 101 | + sequenceNumber.getAndIncrement())) |
104 | .withPeerUpdateFunction((key, intentData) -> getPeerNodes(key, intentData)) | 102 | .withPeerUpdateFunction((key, intentData) -> getPeerNodes(key, intentData)) |
105 | .build(); | 103 | .build(); |
106 | 104 | ||
107 | pendingMap = storageService.<Key, IntentData>eventuallyConsistentMapBuilder() | 105 | pendingMap = storageService.<Key, IntentData>eventuallyConsistentMapBuilder() |
108 | .withName("intent-pending") | 106 | .withName("intent-pending") |
109 | .withSerializer(intentSerializer) | 107 | .withSerializer(intentSerializer) |
110 | - .withTimestampProvider((key, intentData) -> new MultiValuedTimestamp<>(intentData.version(), | 108 | + .withTimestampProvider((key, intentData) -> intentData == null ? |
111 | - System.nanoTime())) | 109 | + new MultiValuedTimestamp<>(new WallClockTimestamp(), System.nanoTime()) : |
110 | + new MultiValuedTimestamp<>(intentData.version(), System.nanoTime())) | ||
112 | .withPeerUpdateFunction((key, intentData) -> getPeerNodes(key, intentData)) | 111 | .withPeerUpdateFunction((key, intentData) -> getPeerNodes(key, intentData)) |
113 | .build(); | 112 | .build(); |
114 | 113 | ||
... | @@ -183,8 +182,15 @@ public class GossipIntentStore | ... | @@ -183,8 +182,15 @@ public class GossipIntentStore |
183 | currentMap.put(newData.key(), new IntentData(newData)); | 182 | currentMap.put(newData.key(), new IntentData(newData)); |
184 | } | 183 | } |
185 | 184 | ||
186 | - // if current.put succeeded | 185 | + // Remove the intent data from the pending map if the newData is more |
187 | - pendingMap.remove(newData.key(), newData); | 186 | + // recent or equal to the existing entry. |
187 | + pendingMap.compute(newData.key(), (key, existingValue) -> { | ||
188 | + if (existingValue == null || !existingValue.version().isNewerThan(newData.version())) { | ||
189 | + return null; | ||
190 | + } else { | ||
191 | + return existingValue; | ||
192 | + } | ||
193 | + }); | ||
188 | } | 194 | } |
189 | } | 195 | } |
190 | 196 | ||
... | @@ -252,10 +258,10 @@ public class GossipIntentStore | ... | @@ -252,10 +258,10 @@ public class GossipIntentStore |
252 | 258 | ||
253 | if (data.version() == null) { | 259 | if (data.version() == null) { |
254 | pendingMap.put(data.key(), new IntentData(data.intent(), data.state(), | 260 | pendingMap.put(data.key(), new IntentData(data.intent(), data.state(), |
255 | - new WallClockTimestamp(), clusterService.getLocalNode().id())); | 261 | + new WallClockTimestamp(), clusterService.getLocalNode().id())); |
256 | } else { | 262 | } else { |
257 | pendingMap.put(data.key(), new IntentData(data.intent(), data.state(), | 263 | pendingMap.put(data.key(), new IntentData(data.intent(), data.state(), |
258 | - data.version(), clusterService.getLocalNode().id())); | 264 | + data.version(), clusterService.getLocalNode().id())); |
259 | } | 265 | } |
260 | } | 266 | } |
261 | 267 | ||
... | @@ -282,7 +288,7 @@ public class GossipIntentStore | ... | @@ -282,7 +288,7 @@ public class GossipIntentStore |
282 | final WallClockTimestamp time = new WallClockTimestamp(now - olderThan); | 288 | final WallClockTimestamp time = new WallClockTimestamp(now - olderThan); |
283 | return pendingMap.values().stream() | 289 | return pendingMap.values().stream() |
284 | .filter(data -> data.version().isOlderThan(time) && | 290 | .filter(data -> data.version().isOlderThan(time) && |
285 | - (!localOnly || isMaster(data.key()))) | 291 | + (!localOnly || isMaster(data.key()))) |
286 | .collect(Collectors.toList()); | 292 | .collect(Collectors.toList()); |
287 | } | 293 | } |
288 | 294 | ... | ... |
-
Please register or login to post a comment