Scrubbing store related TODO/FIXMEs
Change-Id: I4e6bf026845bbd5be127ecacd9956d12f3386c9e
diff --git a/core/api/src/main/java/org/onosproject/net/resource/LinkResourceStore.java b/core/api/src/main/java/org/onosproject/net/resource/LinkResourceStore.java
index 0ddf38e..e3e9db8 100644
--- a/core/api/src/main/java/org/onosproject/net/resource/LinkResourceStore.java
+++ b/core/api/src/main/java/org/onosproject/net/resource/LinkResourceStore.java
@@ -51,7 +51,7 @@
* Returns resources allocated for an Intent.
*
* @param intentId the target Intent's ID
- * @return allocated resources
+ * @return allocated resources or null if no resource is allocated
*/
LinkResourceAllocations getAllocations(IntentId intentId);
diff --git a/core/store/dist/src/main/java/org/onosproject/store/cluster/impl/LeadershipManager.java b/core/store/dist/src/main/java/org/onosproject/store/cluster/impl/LeadershipManager.java
index f3c5877..164d807 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/cluster/impl/LeadershipManager.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/cluster/impl/LeadershipManager.java
@@ -57,7 +57,7 @@
// a unexpected error.
private static final int WAIT_BEFORE_RETRY_MS = 2000;
- // TODO: Appropriate Thread pool sizing.
+ // TODO: Make Thread pool size configurable.
private final ScheduledExecutorService threadPool =
Executors.newScheduledThreadPool(25, namedThreads("leadership-manager-%d"));
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
index a376b93..b18462a 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
@@ -98,7 +98,6 @@
import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_ADVERTISE;
import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_REMOVE_REQ;
-// TODO: give me a better name
/**
* Manages inventory of infrastructure devices using gossip protocol to distribute
* information.
@@ -167,6 +166,10 @@
private ScheduledExecutorService backgroundExecutor;
+ // TODO make these anti-entropy parameters configurable
+ private long initialDelaySec = 5;
+ private long periodSec = 5;
+
@Activate
public void activate() {
@@ -189,9 +192,6 @@
backgroundExecutor =
newSingleThreadScheduledExecutor(minPriority(namedThreads("device-bg-%d")));
- // TODO: Make these configurable
- long initialDelaySec = 5;
- long periodSec = 5;
// start anti-entropy thread
backgroundExecutor.scheduleAtFixedRate(new SendAdvertisementTask(),
initialDelaySec, periodSec, TimeUnit.SECONDS);
@@ -412,7 +412,6 @@
}
boolean removed = availableDevices.remove(deviceId);
if (removed) {
- // TODO: broadcast ... DOWN only?
return new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, device, null);
}
return null;
@@ -885,7 +884,7 @@
if (e.getKey().equals(primary)) {
continue;
}
- // TODO: should keep track of Description timestamp
+ // Note: should keep track of Description timestamp in the future
// and only merge conflicting keys when timestamp is newer.
// Currently assuming there will never be a key conflict between
// providers
@@ -913,7 +912,6 @@
ProviderId primary = pickPrimaryPID(descsMap);
DeviceDescriptions primDescs = descsMap.get(primary);
// if no primary, assume not enabled
- // TODO: revisit this default port enabled/disabled behavior
boolean isEnabled = false;
DefaultAnnotations annotations = DefaultAnnotations.builder().build();
@@ -927,7 +925,7 @@
if (e.getKey().equals(primary)) {
continue;
}
- // TODO: should keep track of Description timestamp
+ // Note: should keep track of Description timestamp in the future
// and only merge conflicting keys when timestamp is newer.
// Currently assuming there will never be a key conflict between
// providers
@@ -968,7 +966,6 @@
return providerDescs.get(pid);
}
- // TODO: should we be throwing exception?
private void unicastMessage(NodeId recipient, MessageSubject subject, Object event) throws IOException {
ClusterMessage message = new ClusterMessage(
clusterService.getLocalNode().id(),
@@ -977,7 +974,6 @@
clusterCommunicator.unicast(message, recipient);
}
- // TODO: should we be throwing exception?
private void broadcastMessage(MessageSubject subject, Object event) throws IOException {
ClusterMessage message = new ClusterMessage(
clusterService.getLocalNode().id(),
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java
index 652603f..83fdf98 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java
@@ -17,7 +17,6 @@
import org.onosproject.store.cluster.messaging.MessageSubject;
-// TODO: add prefix to assure uniqueness.
/**
* MessageSubjects used by GossipDeviceStore peer-peer communication.
*/
diff --git a/core/store/dist/src/main/java/org/onosproject/store/flow/impl/DistributedFlowRuleStore.java b/core/store/dist/src/main/java/org/onosproject/store/flow/impl/DistributedFlowRuleStore.java
index a3de72b..6d2dcf3 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/flow/impl/DistributedFlowRuleStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/flow/impl/DistributedFlowRuleStore.java
@@ -503,7 +503,7 @@
}
// TODO: Confirm if this behavior is correct. See SimpleFlowRuleStore
- // TODO: also update backup.
+ // TODO: also update backup if the behavior is correct.
flowEntries.put(did, new DefaultFlowEntry(rule));
} finally {
flowEntriesLock.writeLock().unlock();
@@ -541,7 +541,7 @@
Future<byte[]> responseFuture = clusterCommunicator.sendAndReceive(message, replicaInfo.master().get());
return SERIALIZER.decode(responseFuture.get(FLOW_RULE_STORE_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS));
} catch (IOException | TimeoutException | ExecutionException | InterruptedException e) {
- // FIXME: throw a FlowStoreException
+ // TODO: Retry against latest master or throw a FlowStoreException
throw new RuntimeException(e);
}
}
@@ -586,8 +586,6 @@
for (Entry<FlowId, ImmutableList<StoredFlowEntry>> e
: backupFlowTable.entrySet()) {
- // TODO: should we be directly updating internal structure or
- // should we be triggering event?
log.trace("loading {}", e.getValue());
for (StoredFlowEntry entry : e.getValue()) {
flowEntries.remove(did, entry);
@@ -714,7 +712,7 @@
// This node is no longer the master holder,
// clean local structure
removeFromPrimary(did);
- // FIXME: probably should stop pending backup activities in
+ // TODO: probably should stop pending backup activities in
// executors to avoid overwriting with old value
}
break;
@@ -767,7 +765,6 @@
} else {
success = backupFlowTable.replace(id, original, newValue);
}
- // TODO retry?
if (!success) {
log.error("Updating backup failed.");
}
@@ -790,7 +787,6 @@
} else {
success = backupFlowTable.replace(id, original, newValue);
}
- // TODO retry?
if (!success) {
log.error("Updating backup failed.");
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/host/impl/GossipHostStore.java b/core/store/dist/src/main/java/org/onosproject/store/host/impl/GossipHostStore.java
index 2354340..10bf9ec 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/host/impl/GossipHostStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/host/impl/GossipHostStore.java
@@ -88,7 +88,6 @@
import com.google.common.collect.Multimaps;
import com.google.common.collect.SetMultimap;
-//TODO: multi-provider, annotation not supported.
/**
* Manages inventory of end-station hosts in distributed data store
* that uses optimistic replication and gossip based techniques.
diff --git a/core/store/dist/src/main/java/org/onosproject/store/hz/AbsentInvalidatingLoadingCache.java b/core/store/dist/src/main/java/org/onosproject/store/hz/AbsentInvalidatingLoadingCache.java
index 1dd0d19..545edda 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/hz/AbsentInvalidatingLoadingCache.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/hz/AbsentInvalidatingLoadingCache.java
@@ -86,6 +86,4 @@
}
return v;
}
-
- // TODO should we be also checking getAll, etc.
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/hz/SQueue.java b/core/store/dist/src/main/java/org/onosproject/store/hz/SQueue.java
index fd184a7..3a2abb8 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/hz/SQueue.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/hz/SQueue.java
@@ -30,8 +30,6 @@
import static com.google.common.base.Preconditions.checkNotNull;
-// TODO: implementation is incomplete
-
/**
* Wrapper around IQueue<byte[]> which serializes/deserializes
* key and value using StoreSerializer.
diff --git a/core/store/dist/src/main/java/org/onosproject/store/hz/STxMap.java b/core/store/dist/src/main/java/org/onosproject/store/hz/STxMap.java
index 4fd4e3c..3a92745 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/hz/STxMap.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/hz/STxMap.java
@@ -28,7 +28,6 @@
import com.hazelcast.core.TransactionalMap;
import com.hazelcast.query.Predicate;
-// TODO: implement Predicate, etc. if we need them.
/**
* Wrapper around TransactionalMap<byte[], byte[]> which serializes/deserializes
* key and value using StoreSerializer.
@@ -100,7 +99,6 @@
@Override
public V getForUpdate(Object key) {
- // TODO Auto-generated method stub
return deserializeVal(m.getForUpdate(serializeKey(key)));
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/intent/impl/DistributedIntentStore.java b/core/store/dist/src/main/java/org/onosproject/store/intent/impl/DistributedIntentStore.java
index 5290131..556f4cb 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/intent/impl/DistributedIntentStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/intent/impl/DistributedIntentStore.java
@@ -90,7 +90,7 @@
private static final String STATES_TABLE = "intent-states";
private CMap<IntentId, IntentState> states;
- // TODO left behind transient state issue: ONOS-103
+ // TODO transient state issue remains for this impl.: ONOS-103
// Map to store instance local intermediate state transition
private transient Map<IntentId, IntentState> transientStates = new ConcurrentHashMap<>();
@@ -142,7 +142,7 @@
getIntentTimer = createResponseTimer("getIntent");
getIntentStateTimer = createResponseTimer("getIntentState");
- // FIXME: We need a way to add serializer for intents which has been plugged-in.
+ // We need a way to add serializer for intents which has been plugged-in.
// As a short term workaround, relax Kryo config to
// registrationRequired=false
serializer = new KryoSerializer() {
@@ -264,7 +264,6 @@
}
}
- // FIXME temporary workaround until we fix our state machine
private void verify(boolean expression, String errorMessageTemplate, Object... errorMessageArgs) {
if (onlyLogTransitionError) {
if (!expression) {
@@ -488,7 +487,6 @@
return failed;
} else {
// everything failed
- // FIXME what to do with events?
return batch.operations();
}
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/link/impl/GossipLinkStore.java b/core/store/dist/src/main/java/org/onosproject/store/link/impl/GossipLinkStore.java
index 5e124d0..e44737c 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/link/impl/GossipLinkStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/link/impl/GossipLinkStore.java
@@ -529,7 +529,7 @@
continue;
}
- // TODO: should keep track of Description timestamp
+ // Note: In the long run we should keep track of Description timestamp
// and only merge conflicting keys when timestamp is newer
// Currently assuming there will never be a key conflict between
// providers
diff --git a/core/store/dist/src/main/java/org/onosproject/store/mastership/impl/DistributedMastershipStore.java b/core/store/dist/src/main/java/org/onosproject/store/mastership/impl/DistributedMastershipStore.java
index 9e1835d..5f98d79 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/mastership/impl/DistributedMastershipStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/mastership/impl/DistributedMastershipStore.java
@@ -299,7 +299,7 @@
} else {
// no master candidate
roleMap.put(deviceId, rv);
- // TODO: Should there be new event type for no MASTER?
+ // TBD: Should there be new event type for no MASTER?
return new MastershipEvent(MASTER_CHANGED, deviceId, rv.roleInfo());
}
case STANDBY:
diff --git a/core/store/dist/src/main/java/org/onosproject/store/resource/impl/DistributedLinkResourceStore.java b/core/store/dist/src/main/java/org/onosproject/store/resource/impl/DistributedLinkResourceStore.java
index 0d7f4f9..2e7dfa1 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/resource/impl/DistributedLinkResourceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/resource/impl/DistributedLinkResourceStore.java
@@ -78,7 +78,6 @@
private final Logger log = getLogger(getClass());
- // FIXME: what is the Bandwidth unit?
private static final Bandwidth DEFAULT_BANDWIDTH = Bandwidth.valueOf(1_000);
// table to store current allocations
@@ -143,7 +142,6 @@
}
private Set<? extends ResourceAllocation> getResourceCapacity(ResourceType type, Link link) {
- // TODO: plugin/provider mechanism to add resource type in the future?
if (type == ResourceType.BANDWIDTH) {
return ImmutableSet.of(getBandwidthResourceCapacity(link));
}
@@ -154,7 +152,6 @@
}
private Set<LambdaResourceAllocation> getLambdaResourceCapacity(Link link) {
- // FIXME enumerate all the possible link/port lambdas
Set<LambdaResourceAllocation> allocations = new HashSet<>();
try {
final int waves = Integer.parseInt(link.annotations().value(wavesAnnotation));
@@ -323,7 +320,6 @@
if (log.isDebugEnabled()) {
logFailureDetail(batch, result);
}
- // FIXME throw appropriate exception, with what failed.
checkState(result.isSuccessful(), "Allocation failed");
}
}
@@ -389,7 +385,6 @@
double bwLeft = bw.bandwidth().toDouble();
bwLeft -= ((BandwidthResourceAllocation) req).bandwidth().toDouble();
if (bwLeft < 0) {
- // FIXME throw appropriate Exception
checkState(bwLeft >= 0,
"There's no Bandwidth left on %s. %s",
link, bwLeft);
@@ -399,7 +394,6 @@
// check if allocation should be accepted
if (!avail.contains(req)) {
// requested lambda was not available
- // FIXME throw appropriate exception
checkState(avail.contains(req),
"Allocating %s on %s failed",
req, link);
@@ -433,7 +427,6 @@
final String dbIntentId = toIntentDbKey(intendId);
final Collection<Link> links = allocations.links();
- // TODO: does release must happen in a batch?
boolean success;
do {
Builder tx = BatchWriteRequest.newBuilder();
@@ -476,7 +469,6 @@
checkNotNull(intentId);
VersionedValue vv = databaseService.get(INTENT_ALLOCATIONS, toIntentDbKey(intentId));
if (vv == null) {
- // FIXME: should we return null or LinkResourceAllocations with nothing allocated?
return null;
}
LinkResourceAllocations allocations = decodeIntentAllocations(vv.value());
@@ -486,7 +478,7 @@
private String toLinkDbKey(LinkKey linkid) {
// introduce cache if necessary
return linkid.toString();
- // TODO: Above is irreversible, if we need reverse conversion
+ // Note: Above is irreversible, if we need reverse conversion
// we may need something like below, due to String only limitation
// byte[] bytes = serializer.encode(linkid);
// StringBuilder builder = new StringBuilder(bytes.length * 4);
diff --git a/core/store/dist/src/main/java/org/onosproject/store/resource/impl/HazelcastLinkResourceStore.java b/core/store/dist/src/main/java/org/onosproject/store/resource/impl/HazelcastLinkResourceStore.java
index 1110396..1294f0b 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/resource/impl/HazelcastLinkResourceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/resource/impl/HazelcastLinkResourceStore.java
@@ -76,7 +76,6 @@
private final Logger log = getLogger(getClass());
- // FIXME: what is the Bandwidth unit?
private static final Bandwidth DEFAULT_BANDWIDTH = Bandwidth.valueOf(1_000);
private static final Bandwidth EMPTY_BW = Bandwidth.valueOf(0);
@@ -134,7 +133,6 @@
}
private Set<? extends ResourceAllocation> getResourceCapacity(ResourceType type, Link link) {
- // TODO: plugin/provider mechanism to add resource type in the future?
if (type == ResourceType.BANDWIDTH) {
return ImmutableSet.of(getBandwidthResourceCapacity(link));
}
@@ -145,7 +143,6 @@
}
private Set<LambdaResourceAllocation> getLambdaResourceCapacity(Link link) {
- // FIXME enumerate all the possible link/port lambdas
Set<LambdaResourceAllocation> allocations = new HashSet<>();
try {
final int waves = Integer.parseInt(link.annotations().value(wavesAnnotation));
@@ -335,7 +332,6 @@
double bwLeft = bw.bandwidth().toDouble();
bwLeft -= ((BandwidthResourceAllocation) req).bandwidth().toDouble();
if (bwLeft < 0) {
- // FIXME throw appropriate Exception
checkState(bwLeft >= 0,
"There's no Bandwidth left on %s. %s",
link, bwLeft);
@@ -345,7 +341,6 @@
// check if allocation should be accepted
if (!avail.contains(req)) {
// requested lambda was not available
- // FIXME throw appropriate exception
checkState(avail.contains(req),
"Allocating %s on %s failed",
req, link);
@@ -381,7 +376,8 @@
boolean success = false;
do {
- // TODO: smaller tx unit to lower the chance of collisions?
+ // Note: might want to break it down into smaller tx unit
+ // to lower the chance of collisions.
TransactionContext tx = theInstance.newTransactionContext();
tx.beginTransaction();
try {
diff --git a/core/store/dist/src/main/java/org/onosproject/store/service/impl/ClusterMessagingProtocol.java b/core/store/dist/src/main/java/org/onosproject/store/service/impl/ClusterMessagingProtocol.java
index 171ce08..0f7e1bd 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/service/impl/ClusterMessagingProtocol.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/service/impl/ClusterMessagingProtocol.java
@@ -100,7 +100,7 @@
// for snapshot
.register(State.class)
.register(TableMetadata.class)
- // TODO: Move this out ?
+ // TODO: Move this out to API?
.register(TableModificationEvent.class)
.register(TableModificationEvent.Type.class)
.build();
diff --git a/core/store/dist/src/main/java/org/onosproject/store/service/impl/DistributedLockManager.java b/core/store/dist/src/main/java/org/onosproject/store/service/impl/DistributedLockManager.java
index a9b92de..ee0b54f 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/service/impl/DistributedLockManager.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/service/impl/DistributedLockManager.java
@@ -96,13 +96,11 @@
@Override
public void addListener(LockEventListener listener) {
- // FIXME:
throw new UnsupportedOperationException();
}
@Override
public void removeListener(LockEventListener listener) {
- // FIXME:
throw new UnsupportedOperationException();
}