Committed by
Gerrit Code Review
Updates to GossipIntentStore and IntentPerfInstaller
Change-Id: If350a6276d758222f9b6ea25ab78d055321eecac
Showing
3 changed files
with
35 additions
and
24 deletions
| ... | @@ -181,7 +181,6 @@ public class IntentPerfInstaller { | ... | @@ -181,7 +181,6 @@ public class IntentPerfInstaller { |
| 181 | } | 181 | } |
| 182 | 182 | ||
| 183 | public void start() { | 183 | public void start() { |
| 184 | - // TODO perhaps move to start(), but need to call before logConfig | ||
| 185 | // adjust numNeighbors and generate list of neighbors | 184 | // adjust numNeighbors and generate list of neighbors |
| 186 | numNeighbors = Math.min(clusterService.getNodes().size() - 1, numNeighbors); | 185 | numNeighbors = Math.min(clusterService.getNodes().size() - 1, numNeighbors); |
| 187 | 186 | ||
| ... | @@ -232,14 +231,13 @@ public class IntentPerfInstaller { | ... | @@ -232,14 +231,13 @@ public class IntentPerfInstaller { |
| 232 | node1.toString().compareTo(node2.toString())); | 231 | node1.toString().compareTo(node2.toString())); |
| 233 | // rotate the local node to index 0 | 232 | // rotate the local node to index 0 |
| 234 | Collections.rotate(nodes, -1 * nodes.indexOf(clusterService.getLocalNode().id())); | 233 | Collections.rotate(nodes, -1 * nodes.indexOf(clusterService.getLocalNode().id())); |
| 235 | - log.info("neighbors (raw): {}", nodes); //TODO remove | 234 | + log.debug("neighbors (raw): {}", nodes); //TODO remove |
| 236 | // generate the sub-list that will contain local node and selected neighbors | 235 | // generate the sub-list that will contain local node and selected neighbors |
| 237 | nodes = nodes.subList(0, numNeighbors + 1); | 236 | nodes = nodes.subList(0, numNeighbors + 1); |
| 238 | - log.info("neighbors: {}", nodes); //TODO remove | 237 | + log.debug("neighbors: {}", nodes); //TODO remove |
| 239 | return nodes; | 238 | return nodes; |
| 240 | } | 239 | } |
| 241 | 240 | ||
| 242 | - | ||
| 243 | private Intent createIntent(Key key, long mac, NodeId node, Multimap<NodeId, Device> devices) { | 241 | private Intent createIntent(Key key, long mac, NodeId node, Multimap<NodeId, Device> devices) { |
| 244 | // choose a random device for which this node is master | 242 | // choose a random device for which this node is master |
| 245 | List<Device> deviceList = devices.get(node).stream().collect(Collectors.toList()); | 243 | List<Device> deviceList = devices.get(node).stream().collect(Collectors.toList()); |
| ... | @@ -309,8 +307,6 @@ public class IntentPerfInstaller { | ... | @@ -309,8 +307,6 @@ public class IntentPerfInstaller { |
| 309 | checkState(intents.values().size() == numberOfKeys, | 307 | checkState(intents.values().size() == numberOfKeys, |
| 310 | "Generated wrong number of intents"); | 308 | "Generated wrong number of intents"); |
| 311 | log.info("Created {} intents", numberOfKeys); | 309 | log.info("Created {} intents", numberOfKeys); |
| 312 | - | ||
| 313 | - //FIXME remove this | ||
| 314 | intents.keySet().forEach(node -> log.info("\t{}\t{}", node, intents.get(node).size())); | 310 | intents.keySet().forEach(node -> log.info("\t{}\t{}", node, intents.get(node).size())); |
| 315 | 311 | ||
| 316 | return Sets.newHashSet(intents.values()); | 312 | return Sets.newHashSet(intents.values()); |
| ... | @@ -336,7 +332,11 @@ public class IntentPerfInstaller { | ... | @@ -336,7 +332,11 @@ public class IntentPerfInstaller { |
| 336 | public void run() { | 332 | public void run() { |
| 337 | prime(); | 333 | prime(); |
| 338 | while (!stopped) { | 334 | while (!stopped) { |
| 339 | - cycle(); | 335 | + try { |
| 336 | + cycle(); | ||
| 337 | + } catch (Exception e) { | ||
| 338 | + log.warn("Exception during cycle", e); | ||
| 339 | + } | ||
| 340 | } | 340 | } |
| 341 | clear(); | 341 | clear(); |
| 342 | } | 342 | } |
| ... | @@ -402,14 +402,19 @@ public class IntentPerfInstaller { | ... | @@ -402,14 +402,19 @@ public class IntentPerfInstaller { |
| 402 | 402 | ||
| 403 | int cycleCount = 0; | 403 | int cycleCount = 0; |
| 404 | private void adjustRates() { | 404 | private void adjustRates() { |
| 405 | + | ||
| 406 | + int addDelta = Math.max(1000 - cycleCount, 10); | ||
| 407 | + double multRatio = Math.min(0.8 + cycleCount * 0.0002, 0.995); | ||
| 408 | + | ||
| 405 | //FIXME need to iron out the rate adjustment | 409 | //FIXME need to iron out the rate adjustment |
| 406 | //FIXME we should taper the adjustments over time | 410 | //FIXME we should taper the adjustments over time |
| 411 | + //FIXME don't just use the lastDuration, take an average | ||
| 407 | if (++cycleCount % 5 == 0) { //TODO: maybe use a timer (we should do this every 5-10 sec) | 412 | if (++cycleCount % 5 == 0) { //TODO: maybe use a timer (we should do this every 5-10 sec) |
| 408 | if (listener.requestThroughput() - listener.processedThroughput() <= 2000 && //was 500 | 413 | if (listener.requestThroughput() - listener.processedThroughput() <= 2000 && //was 500 |
| 409 | lastDuration <= cyclePeriod) { | 414 | lastDuration <= cyclePeriod) { |
| 410 | - lastCount = Math.min(lastCount + 1000, intents.size() / 2); | 415 | + lastCount = Math.min(lastCount + addDelta, intents.size() / 2); |
| 411 | } else { | 416 | } else { |
| 412 | - lastCount *= 0.8; | 417 | + lastCount *= multRatio; |
| 413 | } | 418 | } |
| 414 | log.info("last count: {}, last duration: {} ms (sub: {} vs inst: {})", | 419 | log.info("last count: {}, last duration: {} ms (sub: {} vs inst: {})", |
| 415 | lastCount, lastDuration, listener.requestThroughput(), listener.processedThroughput()); | 420 | lastCount, lastDuration, listener.requestThroughput(), listener.processedThroughput()); | ... | ... |
| ... | @@ -653,7 +653,6 @@ public class DistributedFlowRuleStore | ... | @@ -653,7 +653,6 @@ public class DistributedFlowRuleStore |
| 653 | switch (event.type()) { | 653 | switch (event.type()) { |
| 654 | case MASTER_CHANGED: | 654 | case MASTER_CHANGED: |
| 655 | if (local.equals(rInfo.master().orNull())) { | 655 | if (local.equals(rInfo.master().orNull())) { |
| 656 | - log.info("{} is now the master for {}. Will load flow rules from backup", local, did); | ||
| 657 | // This node is the new master, populate local structure | 656 | // This node is the new master, populate local structure |
| 658 | // from backup | 657 | // from backup |
| 659 | flowTable.loadFromBackup(did); | 658 | flowTable.loadFromBackup(did); |
| ... | @@ -725,6 +724,7 @@ public class DistributedFlowRuleStore | ... | @@ -725,6 +724,7 @@ public class DistributedFlowRuleStore |
| 725 | if (!backupsEnabled) { | 724 | if (!backupsEnabled) { |
| 726 | return; | 725 | return; |
| 727 | } | 726 | } |
| 727 | + log.info("We are now the master for {}. Will load flow rules from backup", deviceId); | ||
| 728 | 728 | ||
| 729 | ConcurrentMap<FlowId, Set<StoredFlowEntry>> flowTable = new ConcurrentHashMap<>(); | 729 | ConcurrentMap<FlowId, Set<StoredFlowEntry>> flowTable = new ConcurrentHashMap<>(); |
| 730 | 730 | ... | ... |
| ... | @@ -16,6 +16,7 @@ | ... | @@ -16,6 +16,7 @@ |
| 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 | +import org.apache.commons.lang.math.RandomUtils; | ||
| 19 | import org.apache.felix.scr.annotations.Activate; | 20 | import org.apache.felix.scr.annotations.Activate; |
| 20 | import org.apache.felix.scr.annotations.Component; | 21 | import org.apache.felix.scr.annotations.Component; |
| 21 | import org.apache.felix.scr.annotations.Deactivate; | 22 | import org.apache.felix.scr.annotations.Deactivate; |
| ... | @@ -45,9 +46,7 @@ import org.onosproject.store.impl.WallClockTimestamp; | ... | @@ -45,9 +46,7 @@ import org.onosproject.store.impl.WallClockTimestamp; |
| 45 | import org.onosproject.store.serializers.KryoNamespaces; | 46 | import org.onosproject.store.serializers.KryoNamespaces; |
| 46 | import org.slf4j.Logger; | 47 | import org.slf4j.Logger; |
| 47 | 48 | ||
| 48 | -import java.util.ArrayList; | ||
| 49 | import java.util.Collection; | 49 | import java.util.Collection; |
| 50 | -import java.util.Collections; | ||
| 51 | import java.util.List; | 50 | import java.util.List; |
| 52 | import java.util.Objects; | 51 | import java.util.Objects; |
| 53 | import java.util.stream.Collectors; | 52 | import java.util.stream.Collectors; |
| ... | @@ -102,7 +101,7 @@ public class GossipIntentStore | ... | @@ -102,7 +101,7 @@ public class GossipIntentStore |
| 102 | pendingMap = new EventuallyConsistentMapImpl<>("intent-pending", | 101 | pendingMap = new EventuallyConsistentMapImpl<>("intent-pending", |
| 103 | clusterService, | 102 | clusterService, |
| 104 | clusterCommunicator, | 103 | clusterCommunicator, |
| 105 | - intentSerializer, // TODO | 104 | + intentSerializer, |
| 106 | new IntentDataClockManager<>(), | 105 | new IntentDataClockManager<>(), |
| 107 | (key, intentData) -> getPeerNodes(key, intentData)); | 106 | (key, intentData) -> getPeerNodes(key, intentData)); |
| 108 | 107 | ||
| ... | @@ -254,29 +253,36 @@ public class GossipIntentStore | ... | @@ -254,29 +253,36 @@ public class GossipIntentStore |
| 254 | private Collection<NodeId> getPeerNodes(Key key, IntentData data) { | 253 | private Collection<NodeId> getPeerNodes(Key key, IntentData data) { |
| 255 | NodeId master = partitionService.getLeader(key); | 254 | NodeId master = partitionService.getLeader(key); |
| 256 | NodeId origin = (data != null) ? data.origin() : null; | 255 | NodeId origin = (data != null) ? data.origin() : null; |
| 256 | + if (master == null || origin == null) { | ||
| 257 | + log.warn("Intent {} has no home; master = {}, origin = {}", | ||
| 258 | + data.key(), master, origin); | ||
| 259 | + } | ||
| 260 | + | ||
| 257 | NodeId me = clusterService.getLocalNode().id(); | 261 | NodeId me = clusterService.getLocalNode().id(); |
| 258 | boolean isMaster = Objects.equals(master, me); | 262 | boolean isMaster = Objects.equals(master, me); |
| 259 | boolean isOrigin = Objects.equals(origin, me); | 263 | boolean isOrigin = Objects.equals(origin, me); |
| 260 | if (isMaster && isOrigin) { | 264 | if (isMaster && isOrigin) { |
| 261 | - return ImmutableList.of(getRandomNode()); | 265 | + return getRandomNode(); |
| 262 | } else if (isMaster) { | 266 | } else if (isMaster) { |
| 263 | - return origin != null ? ImmutableList.of(origin) : ImmutableList.of(getRandomNode()); | 267 | + return origin != null ? ImmutableList.of(origin) : getRandomNode(); |
| 264 | } else if (isOrigin) { | 268 | } else if (isOrigin) { |
| 265 | - return ImmutableList.of(master); | 269 | + return master != null ? ImmutableList.of(master) : getRandomNode(); |
| 266 | } else { | 270 | } else { |
| 267 | - // FIXME: why are we here? log error? | 271 | + log.warn("Not master or origin for intent {}", data.key()); |
| 268 | return ImmutableList.of(master); | 272 | return ImmutableList.of(master); |
| 269 | } | 273 | } |
| 270 | } | 274 | } |
| 271 | 275 | ||
| 272 | - private NodeId getRandomNode() { | 276 | + private List<NodeId> getRandomNode() { |
| 277 | + NodeId me = clusterService.getLocalNode().id(); | ||
| 273 | List<NodeId> nodes = clusterService.getNodes().stream() | 278 | List<NodeId> nodes = clusterService.getNodes().stream() |
| 274 | - .map(ControllerNode::id) | 279 | + .map(ControllerNode::id) |
| 275 | - .collect(Collectors.toCollection(ArrayList::new)); | 280 | + .filter(node -> !Objects.equals(node, me)) |
| 276 | - Collections.shuffle(nodes); | 281 | + .collect(Collectors.toList()); |
| 277 | - // FIXME check if self | 282 | + if (nodes.size() == 0) { |
| 278 | - // FIXME verify nodes.size() > 0 | 283 | + return null; |
| 279 | - return nodes.get(0); | 284 | + } |
| 285 | + return ImmutableList.of(nodes.get(RandomUtils.nextInt(nodes.size()))); | ||
| 280 | } | 286 | } |
| 281 | 287 | ||
| 282 | @Override | 288 | @Override | ... | ... |
-
Please register or login to post a comment