ONOS-6468 Fix for race condition between compilation and installation complete state.
- Fix for a bug where intent compilation complete state,
can overwrites intent installation complete state,
if intent installation completes before compilation complete state is written to the store.
- Introduced internalState version on IntentData,
which is effectively mutation count, in order to avoid
batch write of compilation result overwriting installation result
Change-Id: I5d77dfbe496e690ebdf2b4f9643d2b64c4233182
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/InstallCoordinator.java b/core/net/src/main/java/org/onosproject/net/intent/impl/InstallCoordinator.java
index c21f3f7..c4529bc 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/InstallCoordinator.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/InstallCoordinator.java
@@ -178,18 +178,21 @@
if (errCtxs == null || errCtxs.isEmpty()) {
if (toInstall.isPresent()) {
IntentData installData = toInstall.get();
- log.debug("Completed installing: {}", installData.key());
+ log.debug("Completed installing: {}:{}",
+ installData.key(),
+ installData.intent().id());
installData = new IntentData(installData, installData.installables());
installData.setState(INSTALLED);
intentStore.write(installData);
} else if (toUninstall.isPresent()) {
IntentData uninstallData = toUninstall.get();
uninstallData = new IntentData(uninstallData, Collections.emptyList());
- log.debug("Completed withdrawing: {}", uninstallData.key());
+ log.debug("Completed withdrawing: {}:{}",
+ uninstallData.key(),
+ uninstallData.intent().id());
switch (uninstallData.request()) {
case INSTALL_REQ:
- log.warn("{} was requested to withdraw during installation?",
- uninstallData.intent());
+ // INSTALLED intent was damaged & clean up is now complete
uninstallData.setState(FAILED);
break;
case WITHDRAW_REQ:
@@ -204,16 +207,12 @@
// if toInstall was cause of error, then recompile (manage/increment counter, when exceeded -> CORRUPT)
if (toInstall.isPresent()) {
IntentData installData = toInstall.get();
- installData.setState(CORRUPT);
- installData.incrementErrorCount();
- intentStore.write(installData);
+ intentStore.write(IntentData.corrupt(installData));
}
// if toUninstall was cause of error, then CORRUPT (another job will clean this up)
if (toUninstall.isPresent()) {
IntentData uninstallData = toUninstall.get();
- uninstallData.setState(CORRUPT);
- uninstallData.incrementErrorCount();
- intentStore.write(uninstallData);
+ intentStore.write(IntentData.corrupt(uninstallData));
}
}
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
index 7b09a1a..0cae8da 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
@@ -239,7 +239,7 @@
public void submit(Intent intent) {
checkPermission(INTENT_WRITE);
checkNotNull(intent, INTENT_NULL);
- IntentData data = new IntentData(intent, IntentState.INSTALL_REQ, null);
+ IntentData data = IntentData.submit(intent);
store.addPending(data);
}
@@ -247,7 +247,7 @@
public void withdraw(Intent intent) {
checkPermission(INTENT_WRITE);
checkNotNull(intent, INTENT_NULL);
- IntentData data = new IntentData(intent, IntentState.WITHDRAW_REQ, null);
+ IntentData data = IntentData.withdraw(intent);
store.addPending(data);
}
@@ -255,7 +255,7 @@
public void purge(Intent intent) {
checkPermission(INTENT_WRITE);
checkNotNull(intent, INTENT_NULL);
- IntentData data = new IntentData(intent, IntentState.PURGE_REQ, null);
+ IntentData data = IntentData.purge(intent);
store.addPending(data);
// remove associated group if there is one
@@ -495,6 +495,8 @@
@Override
public void triggerCompile(Iterable<Key> intentKeys,
boolean compileAllFailed) {
+ // TODO figure out who is making excessive calls?
+ log.trace("submitting {} + all?:{}", intentKeys, compileAllFailed);
buildAndSubmitBatches(intentKeys, compileAllFailed);
}
}
@@ -509,6 +511,10 @@
CompletableFuture.runAsync(() -> {
// process intent until the phase reaches one of the final phases
List<CompletableFuture<IntentData>> futures = operations.stream()
+ .map(data -> {
+ log.debug("Start processing of {} {}@{}", data.request(), data.key(), data.version());
+ return data;
+ })
.map(x -> CompletableFuture.completedFuture(x)
.thenApply(IntentManager.this::createInitialPhase)
.thenApplyAsync(IntentProcessPhase::process, workerExecutor)
@@ -525,9 +531,9 @@
case INSTALLING:
case WITHDRAW_REQ:
case WITHDRAWING:
- x.setState(FAILED);
+ // TODO should we swtich based on current
IntentData current = store.getIntentData(x.key());
- return new IntentData(x, current.installables());
+ return IntentData.nextState(current, FAILED);
default:
return null;
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Compiling.java b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Compiling.java
index bb82dfc..124d3fb 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Compiling.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Compiling.java
@@ -57,7 +57,7 @@
List<Intent> compiled = processor.compile(data.intent(),
//TODO consider passing an optional here in the future
stored.map(IntentData::installables).orElse(null));
- return Optional.of(new Installing(processor, new IntentData(data, compiled), stored));
+ return Optional.of(new Installing(processor, IntentData.compiled(data, compiled), stored));
} catch (IntentException e) {
log.warn("Unable to compile intent {} due to:", data.intent(), e);
if (stored.filter(x -> !x.installables().isEmpty()).isPresent()) {
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Failed.java b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Failed.java
index 0a05e4f..26ba858 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Failed.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Failed.java
@@ -33,8 +33,7 @@
* @param data intentData
*/
Failed(IntentData data) {
- this.data = checkNotNull(data);
- this.data.setState(FAILED);
+ this.data = IntentData.nextState(checkNotNull(data), FAILED);
}
@Override
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Withdrawn.java b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Withdrawn.java
index b51f1f7..2e8e67a 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Withdrawn.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/Withdrawn.java
@@ -33,8 +33,7 @@
* @param data intent data containing an intent to be withdrawn
*/
Withdrawn(IntentData data) {
- this.data = checkNotNull(data);
- this.data.setState(WITHDRAWN);
+ this.data = IntentData.nextState(checkNotNull(data), WITHDRAWN);
}
@Override