Adding version stamping/checking to SimpleIntentStore
Change-Id: I08c0bf5e0f5a89275a72fa0900e52ca996942b79
Showing
6 changed files
with
134 additions
and
8 deletions
... | @@ -56,10 +56,28 @@ public class IntentData { //FIXME need to make this "immutable" | ... | @@ -56,10 +56,28 @@ public class IntentData { //FIXME need to make this "immutable" |
56 | return intent.key(); | 56 | return intent.key(); |
57 | } | 57 | } |
58 | 58 | ||
59 | + public Timestamp version() { | ||
60 | + return version; | ||
61 | + } | ||
62 | + | ||
59 | public void setState(IntentState newState) { | 63 | public void setState(IntentState newState) { |
60 | this.state = newState; | 64 | this.state = newState; |
61 | } | 65 | } |
62 | 66 | ||
67 | + /** | ||
68 | + * Sets the version for this intent data. | ||
69 | + * <p> | ||
70 | + * The store should call this method only once when the IntentData is | ||
71 | + * first passed into the pending map. Ideally, an IntentData is timestamped | ||
72 | + * on the same thread that the called used to submit the intents. | ||
73 | + * </p> | ||
74 | + * | ||
75 | + * @param version the version/timestamp for this intent data | ||
76 | + */ | ||
77 | + public void setVersion(Timestamp version) { | ||
78 | + this.version = version; | ||
79 | + } | ||
80 | + | ||
63 | public void setInstallables(List<Intent> installables) { | 81 | public void setInstallables(List<Intent> installables) { |
64 | this.installables = ImmutableList.copyOf(installables); | 82 | this.installables = ImmutableList.copyOf(installables); |
65 | } | 83 | } | ... | ... |
1 | +/* | ||
2 | + * Copyright 2015 Open Networking Laboratory | ||
3 | + * | ||
4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
5 | + * you may not use this file except in compliance with the License. | ||
6 | + * You may obtain a copy of the License at | ||
7 | + * | ||
8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
9 | + * | ||
10 | + * Unless required by applicable law or agreed to in writing, software | ||
11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
13 | + * See the License for the specific language governing permissions and | ||
14 | + * limitations under the License. | ||
15 | + */ | ||
16 | +package org.onosproject.store.impl; | ||
17 | + | ||
18 | +import com.google.common.base.MoreObjects; | ||
19 | +import com.google.common.collect.ComparisonChain; | ||
20 | +import org.onosproject.store.Timestamp; | ||
21 | + | ||
22 | +import java.util.Objects; | ||
23 | + | ||
24 | +import static com.google.common.base.Preconditions.checkArgument; | ||
25 | + | ||
26 | +/** | ||
27 | + * A Timestamp that derives its value from the system clock time (in ns) | ||
28 | + * on the controller where it is generated. | ||
29 | + */ | ||
30 | +public class SystemClockTimestamp implements Timestamp { | ||
31 | + | ||
32 | + private final long unixTimestamp; | ||
33 | + | ||
34 | + public SystemClockTimestamp() { | ||
35 | + unixTimestamp = System.nanoTime(); | ||
36 | + } | ||
37 | + | ||
38 | + @Override | ||
39 | + public int compareTo(Timestamp o) { | ||
40 | + checkArgument(o instanceof SystemClockTimestamp, | ||
41 | + "Must be SystemClockTimestamp", o); | ||
42 | + SystemClockTimestamp that = (SystemClockTimestamp) o; | ||
43 | + | ||
44 | + return ComparisonChain.start() | ||
45 | + .compare(this.unixTimestamp, that.unixTimestamp) | ||
46 | + .result(); | ||
47 | + } | ||
48 | + @Override | ||
49 | + public int hashCode() { | ||
50 | + return Objects.hash(unixTimestamp); | ||
51 | + } | ||
52 | + | ||
53 | + @Override | ||
54 | + public boolean equals(Object obj) { | ||
55 | + if (this == obj) { | ||
56 | + return true; | ||
57 | + } | ||
58 | + if (!(obj instanceof SystemClockTimestamp)) { | ||
59 | + return false; | ||
60 | + } | ||
61 | + SystemClockTimestamp that = (SystemClockTimestamp) obj; | ||
62 | + return Objects.equals(this.unixTimestamp, that.unixTimestamp); | ||
63 | + } | ||
64 | + | ||
65 | + @Override | ||
66 | + public String toString() { | ||
67 | + return MoreObjects.toStringHelper(getClass()) | ||
68 | + .add("unixTimestamp", unixTimestamp) | ||
69 | + .toString(); | ||
70 | + } | ||
71 | + | ||
72 | + public long systemTimestamp() { | ||
73 | + return unixTimestamp; | ||
74 | + } | ||
75 | +} |
... | @@ -71,6 +71,8 @@ import static org.onosproject.net.intent.IntentState.INSTALL_REQ; | ... | @@ -71,6 +71,8 @@ import static org.onosproject.net.intent.IntentState.INSTALL_REQ; |
71 | import static org.onosproject.net.intent.IntentState.WITHDRAWN; | 71 | import static org.onosproject.net.intent.IntentState.WITHDRAWN; |
72 | import static org.slf4j.LoggerFactory.getLogger; | 72 | import static org.slf4j.LoggerFactory.getLogger; |
73 | 73 | ||
74 | +//TODO Note: this store will be removed | ||
75 | + | ||
74 | @Component(immediate = true, enabled = false) | 76 | @Component(immediate = true, enabled = false) |
75 | @Service | 77 | @Service |
76 | public class DistributedIntentStore | 78 | public class DistributedIntentStore | ... | ... |
... | @@ -69,6 +69,8 @@ import static org.onlab.metrics.MetricsUtil.stopTimer; | ... | @@ -69,6 +69,8 @@ import static org.onlab.metrics.MetricsUtil.stopTimer; |
69 | import static org.onosproject.net.intent.IntentState.*; | 69 | import static org.onosproject.net.intent.IntentState.*; |
70 | import static org.slf4j.LoggerFactory.getLogger; | 70 | import static org.slf4j.LoggerFactory.getLogger; |
71 | 71 | ||
72 | +//TODO Note: this store will be removed | ||
73 | + | ||
72 | @Component(immediate = true, enabled = false) | 74 | @Component(immediate = true, enabled = false) |
73 | @Service | 75 | @Service |
74 | public class HazelcastIntentStore | 76 | public class HazelcastIntentStore | ... | ... |
... | @@ -30,6 +30,7 @@ import org.onosproject.net.intent.IntentStore; | ... | @@ -30,6 +30,7 @@ import org.onosproject.net.intent.IntentStore; |
30 | import org.onosproject.net.intent.IntentStoreDelegate; | 30 | import org.onosproject.net.intent.IntentStoreDelegate; |
31 | import org.onosproject.net.intent.Key; | 31 | import org.onosproject.net.intent.Key; |
32 | import org.onosproject.store.AbstractStore; | 32 | import org.onosproject.store.AbstractStore; |
33 | +import org.onosproject.store.impl.SystemClockTimestamp; | ||
33 | import org.slf4j.Logger; | 34 | import org.slf4j.Logger; |
34 | 35 | ||
35 | import java.util.List; | 36 | import java.util.List; |
... | @@ -39,6 +40,8 @@ import java.util.stream.Collectors; | ... | @@ -39,6 +40,8 @@ import java.util.stream.Collectors; |
39 | import static com.google.common.base.Preconditions.checkNotNull; | 40 | import static com.google.common.base.Preconditions.checkNotNull; |
40 | import static org.slf4j.LoggerFactory.getLogger; | 41 | import static org.slf4j.LoggerFactory.getLogger; |
41 | 42 | ||
43 | +//TODO Note: this store will be removed once the GossipIntentStore is stable | ||
44 | + | ||
42 | @Component(immediate = true) | 45 | @Component(immediate = true) |
43 | @Service | 46 | @Service |
44 | //FIXME remove this | 47 | //FIXME remove this |
... | @@ -48,9 +51,8 @@ public class SimpleIntentStore | ... | @@ -48,9 +51,8 @@ public class SimpleIntentStore |
48 | 51 | ||
49 | private final Logger log = getLogger(getClass()); | 52 | private final Logger log = getLogger(getClass()); |
50 | 53 | ||
51 | - // current state maps FIXME.. make this a IntentData map | ||
52 | private final Map<Key, IntentData> current = Maps.newConcurrentMap(); | 54 | private final Map<Key, IntentData> current = Maps.newConcurrentMap(); |
53 | - private final Map<Key, IntentData> pending = Maps.newConcurrentMap(); //String is "key" | 55 | + private final Map<Key, IntentData> pending = Maps.newConcurrentMap(); |
54 | 56 | ||
55 | @Activate | 57 | @Activate |
56 | public void activate() { | 58 | public void activate() { |
... | @@ -160,17 +162,30 @@ public class SimpleIntentStore | ... | @@ -160,17 +162,30 @@ public class SimpleIntentStore |
160 | 162 | ||
161 | @Override | 163 | @Override |
162 | public void write(IntentData newData) { | 164 | public void write(IntentData newData) { |
163 | - //FIXME need to compare the versions | 165 | + synchronized (this) { |
166 | + // TODO this could be refactored/cleaned up | ||
167 | + IntentData currentData = current.get(newData.key()); | ||
168 | + IntentData pendingData = pending.get(newData.key()); | ||
169 | + if (currentData == null || | ||
170 | + // current version is less than or equal to newData's | ||
171 | + // Note: current and newData's versions will be equal for state updates | ||
172 | + currentData.version().compareTo(newData.version()) <= 0) { | ||
164 | current.put(newData.key(), newData); | 173 | current.put(newData.key(), newData); |
174 | + | ||
175 | + if (pendingData != null | ||
176 | + // pendingData version is less than or equal to newData's | ||
177 | + // Note: a new update for this key could be pending (it's version will be greater) | ||
178 | + && pendingData.version().compareTo(newData.version()) <= 0) { | ||
179 | + pending.remove(newData.key()); | ||
180 | + } | ||
181 | + | ||
165 | try { | 182 | try { |
166 | notifyDelegate(IntentEvent.getEvent(newData)); | 183 | notifyDelegate(IntentEvent.getEvent(newData)); |
167 | } catch (IllegalArgumentException e) { | 184 | } catch (IllegalArgumentException e) { |
168 | //no-op | 185 | //no-op |
169 | log.trace("ignore this exception: {}", e); | 186 | log.trace("ignore this exception: {}", e); |
170 | } | 187 | } |
171 | - IntentData old = pending.get(newData.key()); | 188 | + } |
172 | - if (old != null /* && FIXME version check */) { | ||
173 | - pending.remove(newData.key()); | ||
174 | } | 189 | } |
175 | } | 190 | } |
176 | 191 | ||
... | @@ -187,14 +202,26 @@ public class SimpleIntentStore | ... | @@ -187,14 +202,26 @@ public class SimpleIntentStore |
187 | return (data != null) ? data.intent() : null; | 202 | return (data != null) ? data.intent() : null; |
188 | } | 203 | } |
189 | 204 | ||
190 | - | ||
191 | @Override | 205 | @Override |
192 | public void addPending(IntentData data) { | 206 | public void addPending(IntentData data) { |
193 | - //FIXME need to compare versions | 207 | + data.setVersion(new SystemClockTimestamp()); |
208 | + synchronized (this) { | ||
209 | + IntentData existingData = pending.get(data.key()); | ||
210 | + if (existingData == null || | ||
211 | + // existing version is strictly less than data's version | ||
212 | + // Note: if they are equal, we already have the update | ||
213 | + // TODO maybe we should still make this <= to be safe? | ||
214 | + existingData.version().compareTo(data.version()) < 0) { | ||
194 | pending.put(data.key(), data); | 215 | pending.put(data.key(), data); |
195 | checkNotNull(delegate, "Store delegate is not set") | 216 | checkNotNull(delegate, "Store delegate is not set") |
196 | .process(data); | 217 | .process(data); |
197 | notifyDelegate(IntentEvent.getEvent(data)); | 218 | notifyDelegate(IntentEvent.getEvent(data)); |
219 | + } else { | ||
220 | + log.debug("IntentData {} is older than existing: {}", | ||
221 | + data, existingData); | ||
222 | + } | ||
223 | + //TODO consider also checking the current map at this point | ||
224 | + } | ||
198 | } | 225 | } |
199 | 226 | ||
200 | 227 | ... | ... |
... | @@ -39,6 +39,8 @@ import java.util.stream.Collectors; | ... | @@ -39,6 +39,8 @@ import java.util.stream.Collectors; |
39 | import static com.google.common.base.Preconditions.checkNotNull; | 39 | import static com.google.common.base.Preconditions.checkNotNull; |
40 | import static org.slf4j.LoggerFactory.getLogger; | 40 | import static org.slf4j.LoggerFactory.getLogger; |
41 | 41 | ||
42 | +//TODO Note: this store will be removed | ||
43 | + | ||
42 | @Component(immediate = true) | 44 | @Component(immediate = true) |
43 | @Service | 45 | @Service |
44 | public class SimpleIntentStore | 46 | public class SimpleIntentStore | ... | ... |
-
Please register or login to post a comment