Intent manager cleanup
Change-Id: I5a83a69cbaa8f498e5c0ed21588dedf15790d877
Showing
1 changed file
with
4 additions
and
170 deletions
... | @@ -89,7 +89,6 @@ public class IntentManager | ... | @@ -89,7 +89,6 @@ public class IntentManager |
89 | private static final EnumSet<IntentState> RECOMPILE | 89 | private static final EnumSet<IntentState> RECOMPILE |
90 | = EnumSet.of(INSTALL_REQ, FAILED, WITHDRAW_REQ); | 90 | = EnumSet.of(INSTALL_REQ, FAILED, WITHDRAW_REQ); |
91 | 91 | ||
92 | - | ||
93 | // Collections for compiler, installer, and listener are ONOS instance local | 92 | // Collections for compiler, installer, and listener are ONOS instance local |
94 | private final ConcurrentMap<Class<? extends Intent>, | 93 | private final ConcurrentMap<Class<? extends Intent>, |
95 | IntentCompiler<? extends Intent>> compilers = new ConcurrentHashMap<>(); | 94 | IntentCompiler<? extends Intent>> compilers = new ConcurrentHashMap<>(); |
... | @@ -151,7 +150,6 @@ public class IntentManager | ... | @@ -151,7 +150,6 @@ public class IntentManager |
151 | public void submit(Intent intent) { | 150 | public void submit(Intent intent) { |
152 | checkNotNull(intent, INTENT_NULL); | 151 | checkNotNull(intent, INTENT_NULL); |
153 | IntentData data = new IntentData(intent, IntentState.INSTALL_REQ, null); | 152 | IntentData data = new IntentData(intent, IntentState.INSTALL_REQ, null); |
154 | - //FIXME timestamp? | ||
155 | store.addPending(data); | 153 | store.addPending(data); |
156 | } | 154 | } |
157 | 155 | ||
... | @@ -159,7 +157,6 @@ public class IntentManager | ... | @@ -159,7 +157,6 @@ public class IntentManager |
159 | public void withdraw(Intent intent) { | 157 | public void withdraw(Intent intent) { |
160 | checkNotNull(intent, INTENT_NULL); | 158 | checkNotNull(intent, INTENT_NULL); |
161 | IntentData data = new IntentData(intent, IntentState.WITHDRAW_REQ, null); | 159 | IntentData data = new IntentData(intent, IntentState.WITHDRAW_REQ, null); |
162 | - //FIXME timestamp? | ||
163 | store.addPending(data); | 160 | store.addPending(data); |
164 | } | 161 | } |
165 | 162 | ||
... | @@ -290,12 +287,11 @@ public class IntentManager | ... | @@ -290,12 +287,11 @@ public class IntentManager |
290 | List<List<FlowRuleBatchOperation>> plans = new ArrayList<>(installables.size()); | 287 | List<List<FlowRuleBatchOperation>> plans = new ArrayList<>(installables.size()); |
291 | for (Intent installable : installables) { | 288 | for (Intent installable : installables) { |
292 | registerSubclassInstallerIfNeeded(installable); | 289 | registerSubclassInstallerIfNeeded(installable); |
293 | - //FIXME need to migrate installers to FlowRuleOperations | 290 | + //TODO consider migrating installers to FlowRuleOperations |
294 | - // FIXME need to aggregate the FlowRuleOperations across installables | ||
295 | plans.add(getInstaller(installable).install(installable)); | 291 | plans.add(getInstaller(installable).install(installable)); |
296 | } | 292 | } |
297 | 293 | ||
298 | - return merge(plans).build(new FlowRuleOperationsContext() { // FIXME move this out | 294 | + return merge(plans).build(new FlowRuleOperationsContext() { // TODO move this out |
299 | @Override | 295 | @Override |
300 | public void onSuccess(FlowRuleOperations ops) { | 296 | public void onSuccess(FlowRuleOperations ops) { |
301 | log.info("Completed installing: {}", pending.key()); | 297 | log.info("Completed installing: {}", pending.key()); |
... | @@ -307,7 +303,7 @@ public class IntentManager | ... | @@ -307,7 +303,7 @@ public class IntentManager |
307 | public void onError(FlowRuleOperations ops) { | 303 | public void onError(FlowRuleOperations ops) { |
308 | log.warn("Failed installation: {} {} on {}", pending.key(), | 304 | log.warn("Failed installation: {} {} on {}", pending.key(), |
309 | pending.intent(), ops); | 305 | pending.intent(), ops); |
310 | - //FIXME store.write(pending.setState(BROKEN)); | 306 | + //TODO store.write(pending.setState(BROKEN)); |
311 | pending.setState(FAILED); | 307 | pending.setState(FAILED); |
312 | store.write(pending); | 308 | store.write(pending); |
313 | } | 309 | } |
... | @@ -346,7 +342,7 @@ public class IntentManager | ... | @@ -346,7 +342,7 @@ public class IntentManager |
346 | } | 342 | } |
347 | 343 | ||
348 | 344 | ||
349 | - // FIXME... needs tests... or maybe it's just perfect | 345 | + // TODO needs tests... or maybe it's just perfect |
350 | private FlowRuleOperations.Builder merge(List<List<FlowRuleBatchOperation>> plans) { | 346 | private FlowRuleOperations.Builder merge(List<List<FlowRuleBatchOperation>> plans) { |
351 | FlowRuleOperations.Builder builder = FlowRuleOperations.builder(); | 347 | FlowRuleOperations.Builder builder = FlowRuleOperations.builder(); |
352 | // Build a batch one stage at a time | 348 | // Build a batch one stage at a time |
... | @@ -622,166 +618,4 @@ public class IntentManager | ... | @@ -622,166 +618,4 @@ public class IntentManager |
622 | // TODO ensure that only one batch is in flight at a time | 618 | // TODO ensure that only one batch is in flight at a time |
623 | } | 619 | } |
624 | } | 620 | } |
625 | - | ||
626 | -// /////////**************************/////////////////// | ||
627 | -// FIXME Need to build and monitor contexts from FlowRuleService | ||
628 | -// | ||
629 | -// // TODO: better naming | ||
630 | -// private class IntentBatchApplyFirst extends IntentBatchPreprocess { | ||
631 | -// | ||
632 | -// protected final List<CompletedIntentUpdate> intentUpdates; | ||
633 | -// protected final int installAttempt; | ||
634 | -// protected Future<CompletedBatchOperation> future; | ||
635 | -// | ||
636 | -// IntentBatchApplyFirst(Collection<IntentData> operations, List<CompletedIntentUpdate> intentUpdates, | ||
637 | -// long endTime, int installAttempt, Future<CompletedBatchOperation> future) { | ||
638 | -// super(operations, endTime); | ||
639 | -// this.intentUpdates = ImmutableList.copyOf(intentUpdates); | ||
640 | -// this.future = future; | ||
641 | -// this.installAttempt = installAttempt; | ||
642 | -// } | ||
643 | -// | ||
644 | -// @Override | ||
645 | -// public void run() { | ||
646 | -// Future<CompletedBatchOperation> future = applyNextBatch(intentUpdates); | ||
647 | -// new IntentBatchProcessFutures(data, intentUpdates, endTime, installAttempt, future).run(); | ||
648 | -// } | ||
649 | -// | ||
650 | -// /** | ||
651 | -// * Builds and applies the next batch, and returns the future. | ||
652 | -// * | ||
653 | -// * @return Future for next batch | ||
654 | -// */ | ||
655 | -// protected Future<CompletedBatchOperation> applyNextBatch(List<CompletedIntentUpdate> updates) { | ||
656 | -// //TODO test this. (also, maybe save this batch) | ||
657 | -// | ||
658 | -// FlowRuleBatchOperation batch = createFlowRuleBatchOperation(updates); | ||
659 | -// if (batch.size() > 0) { | ||
660 | -// //FIXME apply batch might throw an exception | ||
661 | -// return flowRuleService.applyBatch(batch); | ||
662 | -// } else { | ||
663 | -// return null; | ||
664 | -// } | ||
665 | -// } | ||
666 | -// | ||
667 | -// private FlowRuleBatchOperation createFlowRuleBatchOperation(List<CompletedIntentUpdate> intentUpdates) { | ||
668 | -// FlowRuleBatchOperation batch = new FlowRuleBatchOperation(Collections.emptyList(), null, 0); | ||
669 | -// for (CompletedIntentUpdate update : intentUpdates) { | ||
670 | -// FlowRuleBatchOperation currentBatch = update.currentBatch(); | ||
671 | -// if (currentBatch != null) { | ||
672 | -// batch.addAll(currentBatch); | ||
673 | -// } | ||
674 | -// } | ||
675 | -// return batch; | ||
676 | -// } | ||
677 | -// | ||
678 | -// protected void abandonShip() { | ||
679 | -// // the batch has failed | ||
680 | -// // TODO: maybe we should do more? | ||
681 | -// log.error("Walk the plank, matey..."); | ||
682 | -// future = null; | ||
683 | -// //FIXME | ||
684 | -// //batchService.removeIntentOperations(data); | ||
685 | -// } | ||
686 | -// } | ||
687 | -// | ||
688 | -// // TODO: better naming | ||
689 | -// private class IntentBatchProcessFutures extends IntentBatchApplyFirst { | ||
690 | -// | ||
691 | -// IntentBatchProcessFutures(Collection<IntentData> operations, List<CompletedIntentUpdate> intentUpdates, | ||
692 | -// long endTime, int installAttempt, Future<CompletedBatchOperation> future) { | ||
693 | -// super(operations, intentUpdates, endTime, installAttempt, future); | ||
694 | -// } | ||
695 | -// | ||
696 | -// @Override | ||
697 | -// public void run() { | ||
698 | -// try { | ||
699 | -// Future<CompletedBatchOperation> future = processFutures(); | ||
700 | -// if (future == null) { | ||
701 | -// // there are no outstanding batches; we are done | ||
702 | -// //FIXME | ||
703 | -// return; //? | ||
704 | -// //batchService.removeIntentOperations(data); | ||
705 | -// } else if (System.currentTimeMillis() > endTime) { | ||
706 | -// // - cancel current FlowRuleBatch and resubmit again | ||
707 | -// retry(); | ||
708 | -// } else { | ||
709 | -// // we are not done yet, yield the thread by resubmitting ourselves | ||
710 | -// batchExecutor.submit(new IntentBatchProcessFutures(data, intentUpdates, endTime, | ||
711 | -// installAttempt, future)); | ||
712 | -// } | ||
713 | -// } catch (Exception e) { | ||
714 | -// log.error("Error submitting batches:", e); | ||
715 | -// // FIXME incomplete Intents should be cleaned up | ||
716 | -// // (transition to FAILED, etc.) | ||
717 | -// abandonShip(); | ||
718 | -// } | ||
719 | -// } | ||
720 | -// | ||
721 | -// /** | ||
722 | -// * Iterate through the pending futures, and remove them when they have completed. | ||
723 | -// */ | ||
724 | -// private Future<CompletedBatchOperation> processFutures() { | ||
725 | -// try { | ||
726 | -// CompletedBatchOperation completed = future.get(100, TimeUnit.NANOSECONDS); | ||
727 | -// updateBatches(completed); | ||
728 | -// return applyNextBatch(intentUpdates); | ||
729 | -// } catch (TimeoutException | InterruptedException te) { | ||
730 | -// log.trace("Installation of intents are still pending: {}", data); | ||
731 | -// return future; | ||
732 | -// } catch (ExecutionException e) { | ||
733 | -// log.warn("Execution of batch failed: {}", data, e); | ||
734 | -// abandonShip(); | ||
735 | -// return future; | ||
736 | -// } | ||
737 | -// } | ||
738 | -// | ||
739 | -// private void updateBatches(CompletedBatchOperation completed) { | ||
740 | -// if (completed.isSuccess()) { | ||
741 | -// for (CompletedIntentUpdate update : intentUpdates) { | ||
742 | -// update.batchSuccess(); | ||
743 | -// } | ||
744 | -// } else { | ||
745 | -// // entire batch has been reverted... | ||
746 | -// log.debug("Failed items: {}", completed.failedItems()); | ||
747 | -// log.debug("Failed ids: {}", completed.failedIds()); | ||
748 | -// | ||
749 | -// for (Long id : completed.failedIds()) { | ||
750 | -// IntentId targetId = IntentId.valueOf(id); | ||
751 | -// for (CompletedIntentUpdate update : intentUpdates) { | ||
752 | -// for (Intent intent : update.allInstallables()) { | ||
753 | -// if (intent.id().equals(targetId)) { | ||
754 | -// update.batchFailed(); | ||
755 | -// break; | ||
756 | -// } | ||
757 | -// } | ||
758 | -// } | ||
759 | -// // don't increment the non-failed items, as they have been reverted. | ||
760 | -// } | ||
761 | -// } | ||
762 | -// } | ||
763 | -// | ||
764 | -// private void retry() { | ||
765 | -// log.debug("Execution timed out, retrying."); | ||
766 | -// if (future.cancel(true)) { // cancel success; batch is reverted | ||
767 | -// // reset the timer | ||
768 | -// long timeLimit = calculateTimeoutLimit(); | ||
769 | -// int attempts = installAttempt + 1; | ||
770 | -// if (attempts == MAX_ATTEMPTS) { | ||
771 | -// log.warn("Install request timed out: {}", data); | ||
772 | -// for (CompletedIntentUpdate update : intentUpdates) { | ||
773 | -// update.batchFailed(); | ||
774 | -// } | ||
775 | -// } else if (attempts > MAX_ATTEMPTS) { | ||
776 | -// abandonShip(); | ||
777 | -// return; | ||
778 | -// } | ||
779 | -// Future<CompletedBatchOperation> future = applyNextBatch(intentUpdates); | ||
780 | -// batchExecutor.submit(new IntentBatchProcessFutures(data, intentUpdates, timeLimit, attempts, future)); | ||
781 | -// } else { | ||
782 | -// log.error("Cancelling FlowRuleBatch failed."); | ||
783 | -// abandonShip(); | ||
784 | -// } | ||
785 | -// } | ||
786 | -// } | ||
787 | } | 621 | } | ... | ... |
-
Please register or login to post a comment