홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Madan Jampani
Committed by Gerrit Code Review
25c3dcb3 1 parent 67a5719e

Removed operation retry wrappers around various consistent map/atomic counter operations 

Change-Id: Ie6c22a983a01bf3488eff51a493554319c5d15f8
Inline Side-by-side
Showing 6 changed files with 13 additions and 51 deletions
...@@ -36,7 +36,6 @@ import org.apache.felix.scr.annotations.Reference; ...@@ -36,7 +36,6 @@ import org.apache.felix.scr.annotations.Reference;
36 import org.apache.felix.scr.annotations.ReferenceCardinality; 36 import org.apache.felix.scr.annotations.ReferenceCardinality;
37 import org.apache.felix.scr.annotations.Service; 37 import org.apache.felix.scr.annotations.Service;
38 import org.onlab.util.KryoNamespace; 38 import org.onlab.util.KryoNamespace;
39 -import org.onlab.util.Tools;
40 import org.onosproject.net.config.Config; 39 import org.onosproject.net.config.Config;
41 import org.onosproject.net.config.ConfigApplyDelegate; 40 import org.onosproject.net.config.ConfigApplyDelegate;
42 import org.onosproject.net.config.ConfigFactory; 41 import org.onosproject.net.config.ConfigFactory;
...@@ -47,7 +46,6 @@ import org.onosproject.net.config.NetworkConfigStoreDelegate; ...@@ -47,7 +46,6 @@ import org.onosproject.net.config.NetworkConfigStoreDelegate;
47 import org.onosproject.store.AbstractStore; 46 import org.onosproject.store.AbstractStore;
48 import org.onosproject.store.serializers.KryoNamespaces; 47 import org.onosproject.store.serializers.KryoNamespaces;
49 import org.onosproject.store.service.ConsistentMap; 48 import org.onosproject.store.service.ConsistentMap;
50 -import org.onosproject.store.service.ConsistentMapException;
51 import org.onosproject.store.service.MapEvent; 49 import org.onosproject.store.service.MapEvent;
52 import org.onosproject.store.service.MapEventListener; 50 import org.onosproject.store.service.MapEventListener;
53 import org.onosproject.store.service.Serializer; 51 import org.onosproject.store.service.Serializer;
...@@ -79,7 +77,6 @@ public class DistributedNetworkConfigStore ...@@ -79,7 +77,6 @@ public class DistributedNetworkConfigStore
79 77
80 private final Logger log = LoggerFactory.getLogger(getClass()); 78 private final Logger log = LoggerFactory.getLogger(getClass());
81 79
82 - private static final int MAX_BACKOFF = 10;
83 private static final String INVALID_CONFIG_JSON = 80 private static final String INVALID_CONFIG_JSON =
84 "JSON node does not contain valid configuration"; 81 "JSON node does not contain valid configuration";
85 private static final String INVALID_JSON_LIST = 82 private static final String INVALID_JSON_LIST =
...@@ -216,9 +213,7 @@ public class DistributedNetworkConfigStore ...@@ -216,9 +213,7 @@ public class DistributedNetworkConfigStore
216 213
217 @Override 214 @Override
218 public <S, T extends Config<S>> T getConfig(S subject, Class<T> configClass) { 215 public <S, T extends Config<S>> T getConfig(S subject, Class<T> configClass) {
219 - // TODO: need to identify and address the root cause for timeouts. 216 + Versioned<JsonNode> json = configs.get(key(subject, configClass));
220 - Versioned<JsonNode> json = Tools.retryable(configs::get, ConsistentMapException.class, 1, MAX_BACKOFF)
221 - .apply(key(subject, configClass));
222 return json != null ? createConfig(subject, configClass, json.value()) : null; 217 return json != null ? createConfig(subject, configClass, json.value()) : null;
223 } 218 }
224 219
......
...@@ -28,7 +28,6 @@ import org.onlab.packet.IpAddress; ...@@ -28,7 +28,6 @@ import org.onlab.packet.IpAddress;
28 import org.onlab.packet.MacAddress; 28 import org.onlab.packet.MacAddress;
29 import org.onlab.packet.VlanId; 29 import org.onlab.packet.VlanId;
30 import org.onlab.util.KryoNamespace; 30 import org.onlab.util.KryoNamespace;
31 -import org.onlab.util.Tools;
32 import org.onosproject.net.Annotations; 31 import org.onosproject.net.Annotations;
33 import org.onosproject.net.ConnectPoint; 32 import org.onosproject.net.ConnectPoint;
34 import org.onosproject.net.DefaultAnnotations; 33 import org.onosproject.net.DefaultAnnotations;
...@@ -45,12 +44,10 @@ import org.onosproject.net.provider.ProviderId; ...@@ -45,12 +44,10 @@ import org.onosproject.net.provider.ProviderId;
45 import org.onosproject.store.AbstractStore; 44 import org.onosproject.store.AbstractStore;
46 import org.onosproject.store.serializers.KryoNamespaces; 45 import org.onosproject.store.serializers.KryoNamespaces;
47 import org.onosproject.store.service.ConsistentMap; 46 import org.onosproject.store.service.ConsistentMap;
48 -import org.onosproject.store.service.ConsistentMapException;
49 import org.onosproject.store.service.MapEvent; 47 import org.onosproject.store.service.MapEvent;
50 import org.onosproject.store.service.MapEventListener; 48 import org.onosproject.store.service.MapEventListener;
51 import org.onosproject.store.service.Serializer; 49 import org.onosproject.store.service.Serializer;
52 import org.onosproject.store.service.StorageService; 50 import org.onosproject.store.service.StorageService;
53 -import org.onosproject.store.service.Versioned;
54 import org.slf4j.Logger; 51 import org.slf4j.Logger;
55 52
56 import java.util.Collection; 53 import java.util.Collection;
...@@ -60,7 +57,6 @@ import java.util.Objects; ...@@ -60,7 +57,6 @@ import java.util.Objects;
60 import java.util.Set; 57 import java.util.Set;
61 import java.util.concurrent.ConcurrentHashMap; 58 import java.util.concurrent.ConcurrentHashMap;
62 import java.util.function.Predicate; 59 import java.util.function.Predicate;
63 -import java.util.function.Supplier;
64 import java.util.stream.Collectors; 60 import java.util.stream.Collectors;
65 61
66 import static com.google.common.base.Preconditions.checkNotNull; 62 import static com.google.common.base.Preconditions.checkNotNull;
...@@ -83,7 +79,7 @@ public class DistributedHostStore ...@@ -83,7 +79,7 @@ public class DistributedHostStore
83 @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) 79 @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
84 protected StorageService storageService; 80 protected StorageService storageService;
85 81
86 - private ConsistentMap<HostId, DefaultHost> host; 82 + private ConsistentMap<HostId, DefaultHost> hostsConsistentMap;
87 private Map<HostId, DefaultHost> hosts; 83 private Map<HostId, DefaultHost> hosts;
88 84
89 private final ConcurrentHashMap<HostId, DefaultHost> prevHosts = 85 private final ConcurrentHashMap<HostId, DefaultHost> prevHosts =
...@@ -97,24 +93,24 @@ public class DistributedHostStore ...@@ -97,24 +93,24 @@ public class DistributedHostStore
97 KryoNamespace.Builder hostSerializer = KryoNamespace.newBuilder() 93 KryoNamespace.Builder hostSerializer = KryoNamespace.newBuilder()
98 .register(KryoNamespaces.API); 94 .register(KryoNamespaces.API);
99 95
100 - host = storageService.<HostId, DefaultHost>consistentMapBuilder() 96 + hostsConsistentMap = storageService.<HostId, DefaultHost>consistentMapBuilder()
101 .withName("onos-hosts") 97 .withName("onos-hosts")
102 .withRelaxedReadConsistency() 98 .withRelaxedReadConsistency()
103 .withSerializer(Serializer.using(hostSerializer.build())) 99 .withSerializer(Serializer.using(hostSerializer.build()))
104 .build(); 100 .build();
105 101
106 - hosts = host.asJavaMap(); 102 + hosts = hostsConsistentMap.asJavaMap();
107 103
108 prevHosts.putAll(hosts); 104 prevHosts.putAll(hosts);
109 105
110 - host.addListener(hostLocationTracker); 106 + hostsConsistentMap.addListener(hostLocationTracker);
111 107
112 log.info("Started"); 108 log.info("Started");
113 } 109 }
114 110
115 @Deactivate 111 @Deactivate
116 public void deactivate() { 112 public void deactivate() {
117 - host.removeListener(hostLocationTracker); 113 + hostsConsistentMap.removeListener(hostLocationTracker);
118 prevHosts.clear(); 114 prevHosts.clear();
119 115
120 log.info("Stopped"); 116 log.info("Stopped");
...@@ -162,8 +158,7 @@ public class DistributedHostStore ...@@ -162,8 +158,7 @@ public class DistributedHostStore
162 HostId hostId, 158 HostId hostId,
163 HostDescription hostDescription, 159 HostDescription hostDescription,
164 boolean replaceIPs) { 160 boolean replaceIPs) {
165 - Supplier<Versioned<DefaultHost>> supplier = 161 + hostsConsistentMap.computeIf(hostId,
166 - () -> host.computeIf(hostId,
167 existingHost -> shouldUpdate(existingHost, providerId, hostId, 162 existingHost -> shouldUpdate(existingHost, providerId, hostId,
168 hostDescription, replaceIPs), 163 hostDescription, replaceIPs),
169 (id, existingHost) -> { 164 (id, existingHost) -> {
...@@ -193,12 +188,6 @@ public class DistributedHostStore ...@@ -193,12 +188,6 @@ public class DistributedHostStore
193 addresses, 188 addresses,
194 annotations); 189 annotations);
195 }); 190 });
196 -
197 - Tools.retryable(supplier,
198 - ConsistentMapException.ConcurrentModification.class,
199 - Integer.MAX_VALUE,
200 - 50).get();
201 -
202 return null; 191 return null;
203 } 192 }
204 193
......
...@@ -42,7 +42,6 @@ import org.onosproject.store.cluster.messaging.MessageSubject; ...@@ -42,7 +42,6 @@ import org.onosproject.store.cluster.messaging.MessageSubject;
42 import org.onosproject.store.serializers.KryoNamespaces; 42 import org.onosproject.store.serializers.KryoNamespaces;
43 import org.onosproject.store.serializers.StoreSerializer; 43 import org.onosproject.store.serializers.StoreSerializer;
44 import org.onosproject.store.service.ConsistentMap; 44 import org.onosproject.store.service.ConsistentMap;
45 -import org.onosproject.store.service.ConsistentMapException;
46 import org.onosproject.store.service.Serializer; 45 import org.onosproject.store.service.Serializer;
47 import org.onosproject.store.service.StorageService; 46 import org.onosproject.store.service.StorageService;
48 import org.osgi.service.component.ComponentContext; 47 import org.osgi.service.component.ComponentContext;
...@@ -60,7 +59,6 @@ import static com.google.common.base.Preconditions.checkArgument; ...@@ -60,7 +59,6 @@ import static com.google.common.base.Preconditions.checkArgument;
60 import static com.google.common.base.Strings.isNullOrEmpty; 59 import static com.google.common.base.Strings.isNullOrEmpty;
61 import static org.onlab.util.Tools.get; 60 import static org.onlab.util.Tools.get;
62 import static org.onlab.util.Tools.groupedThreads; 61 import static org.onlab.util.Tools.groupedThreads;
63 -import static org.onlab.util.Tools.retryable;
64 import static org.slf4j.LoggerFactory.getLogger; 62 import static org.slf4j.LoggerFactory.getLogger;
65 63
66 /** 64 /**
...@@ -206,9 +204,7 @@ public class DistributedPacketStore ...@@ -206,9 +204,7 @@ public class DistributedPacketStore
206 } 204 }
207 205
208 private void add(PacketRequest request) { 206 private void add(PacketRequest request) {
209 - AtomicBoolean firstRequest = 207 + AtomicBoolean firstRequest = addInternal(request);
210 - retryable(this::addInternal, ConsistentMapException.ConcurrentModification.class,
211 - Integer.MAX_VALUE, MAX_BACKOFF).apply(request);
212 if (firstRequest.get() && delegate != null) { 208 if (firstRequest.get() && delegate != null) {
213 // The instance that makes the first request will push to all devices 209 // The instance that makes the first request will push to all devices
214 delegate.requestPackets(request); 210 delegate.requestPackets(request);
...@@ -234,9 +230,7 @@ public class DistributedPacketStore ...@@ -234,9 +230,7 @@ public class DistributedPacketStore
234 } 230 }
235 231
236 private void remove(PacketRequest request) { 232 private void remove(PacketRequest request) {
237 - AtomicBoolean removedLast = 233 + AtomicBoolean removedLast = removeInternal(request);
238 - retryable(this::removeInternal, ConsistentMapException.ConcurrentModification.class,
239 - Integer.MAX_VALUE, MAX_BACKOFF).apply(request);
240 if (removedLast.get() && delegate != null) { 234 if (removedLast.get() && delegate != null) {
241 // The instance that removes the last request will remove from all devices 235 // The instance that removes the last request will remove from all devices
242 delegate.cancelPackets(request); 236 delegate.cancelPackets(request);
......
...@@ -18,7 +18,6 @@ package org.onosproject.store.resource.impl; ...@@ -18,7 +18,6 @@ package org.onosproject.store.resource.impl;
18 import com.google.common.collect.ImmutableList; 18 import com.google.common.collect.ImmutableList;
19 import com.google.common.collect.ImmutableSet; 19 import com.google.common.collect.ImmutableSet;
20 import org.onlab.util.GuavaCollectors; 20 import org.onlab.util.GuavaCollectors;
21 -import org.onlab.util.Tools;
22 import org.onosproject.net.resource.ContinuousResource; 21 import org.onosproject.net.resource.ContinuousResource;
23 import org.onosproject.net.resource.ContinuousResourceId; 22 import org.onosproject.net.resource.ContinuousResourceId;
24 import org.onosproject.net.resource.DiscreteResourceId; 23 import org.onosproject.net.resource.DiscreteResourceId;
...@@ -26,7 +25,6 @@ import org.onosproject.net.resource.Resource; ...@@ -26,7 +25,6 @@ import org.onosproject.net.resource.Resource;
26 import org.onosproject.net.resource.ResourceAllocation; 25 import org.onosproject.net.resource.ResourceAllocation;
27 import org.onosproject.net.resource.ResourceConsumerId; 26 import org.onosproject.net.resource.ResourceConsumerId;
28 import org.onosproject.store.service.ConsistentMap; 27 import org.onosproject.store.service.ConsistentMap;
29 -import org.onosproject.store.service.ConsistentMapException;
30 import org.onosproject.store.service.StorageService; 28 import org.onosproject.store.service.StorageService;
31 import org.onosproject.store.service.TransactionContext; 29 import org.onosproject.store.service.TransactionContext;
32 import org.onosproject.store.service.Versioned; 30 import org.onosproject.store.service.Versioned;
...@@ -36,8 +34,6 @@ import java.util.List; ...@@ -36,8 +34,6 @@ import java.util.List;
36 import java.util.Set; 34 import java.util.Set;
37 import java.util.stream.Stream; 35 import java.util.stream.Stream;
38 36
39 -import static org.onosproject.store.resource.impl.ConsistentResourceStore.MAX_RETRIES;
40 -import static org.onosproject.store.resource.impl.ConsistentResourceStore.RETRY_DELAY;
41 import static org.onosproject.store.resource.impl.ConsistentResourceStore.SERIALIZER; 37 import static org.onosproject.store.resource.impl.ConsistentResourceStore.SERIALIZER;
42 38
43 class ConsistentContinuousResourceSubStore { 39 class ConsistentContinuousResourceSubStore {
...@@ -54,8 +50,7 @@ class ConsistentContinuousResourceSubStore { ...@@ -54,8 +50,7 @@ class ConsistentContinuousResourceSubStore {
54 .withSerializer(SERIALIZER) 50 .withSerializer(SERIALIZER)
55 .build(); 51 .build();
56 52
57 - Tools.retryable(() -> childMap.put(Resource.ROOT.id(), new LinkedHashSet<>()), 53 + childMap.put(Resource.ROOT.id(), new LinkedHashSet<>());
58 - ConsistentMapException.class, MAX_RETRIES, RETRY_DELAY);
59 } 54 }
60 55
61 TransactionalContinuousResourceSubStore transactional(TransactionContext tx) { 56 TransactionalContinuousResourceSubStore transactional(TransactionContext tx) {
......
...@@ -17,7 +17,6 @@ package org.onosproject.store.resource.impl; ...@@ -17,7 +17,6 @@ package org.onosproject.store.resource.impl;
17 17
18 import com.google.common.collect.ImmutableList; 18 import com.google.common.collect.ImmutableList;
19 import com.google.common.collect.ImmutableSet; 19 import com.google.common.collect.ImmutableSet;
20 -import org.onlab.util.Tools;
21 import org.onosproject.net.resource.DiscreteResource; 20 import org.onosproject.net.resource.DiscreteResource;
22 import org.onosproject.net.resource.DiscreteResourceId; 21 import org.onosproject.net.resource.DiscreteResourceId;
23 import org.onosproject.net.resource.Resource; 22 import org.onosproject.net.resource.Resource;
...@@ -25,7 +24,6 @@ import org.onosproject.net.resource.ResourceAllocation; ...@@ -25,7 +24,6 @@ import org.onosproject.net.resource.ResourceAllocation;
25 import org.onosproject.net.resource.ResourceConsumerId; 24 import org.onosproject.net.resource.ResourceConsumerId;
26 import org.onosproject.net.resource.Resources; 25 import org.onosproject.net.resource.Resources;
27 import org.onosproject.store.service.ConsistentMap; 26 import org.onosproject.store.service.ConsistentMap;
28 -import org.onosproject.store.service.ConsistentMapException;
29 import org.onosproject.store.service.StorageService; 27 import org.onosproject.store.service.StorageService;
30 import org.onosproject.store.service.TransactionContext; 28 import org.onosproject.store.service.TransactionContext;
31 import org.onosproject.store.service.Versioned; 29 import org.onosproject.store.service.Versioned;
...@@ -35,8 +33,6 @@ import java.util.Map; ...@@ -35,8 +33,6 @@ import java.util.Map;
35 import java.util.Set; 33 import java.util.Set;
36 import java.util.stream.Stream; 34 import java.util.stream.Stream;
37 35
38 -import static org.onosproject.store.resource.impl.ConsistentResourceStore.MAX_RETRIES;
39 -import static org.onosproject.store.resource.impl.ConsistentResourceStore.RETRY_DELAY;
40 import static org.onosproject.store.resource.impl.ConsistentResourceStore.SERIALIZER; 36 import static org.onosproject.store.resource.impl.ConsistentResourceStore.SERIALIZER;
41 37
42 class ConsistentDiscreteResourceSubStore { 38 class ConsistentDiscreteResourceSubStore {
...@@ -53,8 +49,7 @@ class ConsistentDiscreteResourceSubStore { ...@@ -53,8 +49,7 @@ class ConsistentDiscreteResourceSubStore {
53 .withSerializer(SERIALIZER) 49 .withSerializer(SERIALIZER)
54 .build(); 50 .build();
55 51
56 - Tools.retryable(() -> childMap.put(Resource.ROOT.id(), DiscreteResources.empty()), 52 + childMap.put(Resource.ROOT.id(), DiscreteResources.empty());
57 - ConsistentMapException.class, MAX_RETRIES, RETRY_DELAY);
58 } 53 }
59 54
60 TransactionalDiscreteResourceSubStore transactional(TransactionContext tx) { 55 TransactionalDiscreteResourceSubStore transactional(TransactionContext tx) {
......
...@@ -33,7 +33,6 @@ import org.apache.felix.scr.annotations.Property; ...@@ -33,7 +33,6 @@ import org.apache.felix.scr.annotations.Property;
33 import org.apache.felix.scr.annotations.Reference; 33 import org.apache.felix.scr.annotations.Reference;
34 import org.apache.felix.scr.annotations.ReferenceCardinality; 34 import org.apache.felix.scr.annotations.ReferenceCardinality;
35 import org.onlab.packet.Ethernet; 35 import org.onlab.packet.Ethernet;
36 -import org.onlab.util.Tools;
37 import org.onosproject.cfg.ComponentConfigService; 36 import org.onosproject.cfg.ComponentConfigService;
38 import org.onosproject.cluster.ClusterMetadataService; 37 import org.onosproject.cluster.ClusterMetadataService;
39 import org.onosproject.cluster.ClusterService; 38 import org.onosproject.cluster.ClusterService;
...@@ -70,7 +69,6 @@ import org.onosproject.net.provider.AbstractProvider; ...@@ -70,7 +69,6 @@ import org.onosproject.net.provider.AbstractProvider;
70 import org.onosproject.net.provider.ProviderId; 69 import org.onosproject.net.provider.ProviderId;
71 import org.onosproject.provider.lldpcommon.LinkDiscovery; 70 import org.onosproject.provider.lldpcommon.LinkDiscovery;
72 import org.onosproject.provider.lldpcommon.LinkDiscoveryContext; 71 import org.onosproject.provider.lldpcommon.LinkDiscoveryContext;
73 -import org.onosproject.store.service.ConsistentMapException;
74 import org.osgi.service.component.ComponentContext; 72 import org.osgi.service.component.ComponentContext;
75 import org.slf4j.Logger; 73 import org.slf4j.Logger;
76 74
...@@ -252,14 +250,10 @@ public class LldpLinkProvider extends AbstractProvider implements ProbedLinkProv ...@@ -252,14 +250,10 @@ public class LldpLinkProvider extends AbstractProvider implements ProbedLinkProv
252 cfgRegistry.addListener(cfgListener); 250 cfgRegistry.addListener(cfgListener);
253 factories.forEach(cfgRegistry::registerConfigFactory); 251 factories.forEach(cfgRegistry::registerConfigFactory);
254 252
255 - SuppressionConfig cfg = 253 + SuppressionConfig cfg = cfgRegistry.getConfig(appId, SuppressionConfig.class);
256 - Tools.retryable(() -> cfgRegistry.getConfig(appId, SuppressionConfig.class),
257 - ConsistentMapException.class, MAX_RETRIES, RETRY_DELAY).get();
258 if (cfg == null) { 254 if (cfg == null) {
259 // If no configuration is found, register default. 255 // If no configuration is found, register default.
260 - cfg = Tools.retryable(this::setDefaultSuppressionConfig, 256 + cfg = this.setDefaultSuppressionConfig();
261 - ConsistentMapException.class,
262 - MAX_RETRIES, RETRY_DELAY).get();
263 } 257 }
264 cfgListener.reconfigureSuppressionRules(cfg); 258 cfgListener.reconfigureSuppressionRules(cfg);
265 259
......