ONOS-5101: Ensure app activation order reflects the dependency order
Change-Id: I77e0579436d80643b8262f0ec5ad6efb57936a0b
Showing
1 changed file
with
30 additions
and
11 deletions
... | @@ -30,7 +30,6 @@ import org.apache.felix.scr.annotations.Deactivate; | ... | @@ -30,7 +30,6 @@ import org.apache.felix.scr.annotations.Deactivate; |
30 | import org.apache.felix.scr.annotations.Reference; | 30 | import org.apache.felix.scr.annotations.Reference; |
31 | import org.apache.felix.scr.annotations.ReferenceCardinality; | 31 | import org.apache.felix.scr.annotations.ReferenceCardinality; |
32 | import org.apache.felix.scr.annotations.Service; | 32 | import org.apache.felix.scr.annotations.Service; |
33 | -import org.onlab.util.Tools; | ||
34 | import org.onosproject.app.ApplicationDescription; | 33 | import org.onosproject.app.ApplicationDescription; |
35 | import org.onosproject.app.ApplicationEvent; | 34 | import org.onosproject.app.ApplicationEvent; |
36 | import org.onosproject.app.ApplicationException; | 35 | import org.onosproject.app.ApplicationException; |
... | @@ -49,6 +48,9 @@ import org.onosproject.security.Permission; | ... | @@ -49,6 +48,9 @@ import org.onosproject.security.Permission; |
49 | import org.onosproject.store.cluster.messaging.ClusterCommunicationService; | 48 | import org.onosproject.store.cluster.messaging.ClusterCommunicationService; |
50 | import org.onosproject.store.cluster.messaging.MessageSubject; | 49 | import org.onosproject.store.cluster.messaging.MessageSubject; |
51 | import org.onosproject.store.serializers.KryoNamespaces; | 50 | import org.onosproject.store.serializers.KryoNamespaces; |
51 | +import org.onosproject.store.service.AtomicValue; | ||
52 | +import org.onosproject.store.service.AtomicValueEvent; | ||
53 | +import org.onosproject.store.service.AtomicValueEventListener; | ||
52 | import org.onosproject.store.service.ConsistentMap; | 54 | import org.onosproject.store.service.ConsistentMap; |
53 | import org.onosproject.store.service.MapEvent; | 55 | import org.onosproject.store.service.MapEvent; |
54 | import org.onosproject.store.service.MapEventListener; | 56 | import org.onosproject.store.service.MapEventListener; |
... | @@ -119,6 +121,7 @@ public class DistributedApplicationStore extends ApplicationArchive | ... | @@ -119,6 +121,7 @@ public class DistributedApplicationStore extends ApplicationArchive |
119 | private ExecutorService messageHandlingExecutor; | 121 | private ExecutorService messageHandlingExecutor; |
120 | 122 | ||
121 | private ConsistentMap<ApplicationId, InternalApplicationHolder> apps; | 123 | private ConsistentMap<ApplicationId, InternalApplicationHolder> apps; |
124 | + private AtomicValue<Application> nextAppToActivate; | ||
122 | 125 | ||
123 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) | 126 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) |
124 | protected ClusterCommunicationService clusterCommunicator; | 127 | protected ClusterCommunicationService clusterCommunicator; |
... | @@ -133,6 +136,7 @@ public class DistributedApplicationStore extends ApplicationArchive | ... | @@ -133,6 +136,7 @@ public class DistributedApplicationStore extends ApplicationArchive |
133 | protected ApplicationIdStore idStore; | 136 | protected ApplicationIdStore idStore; |
134 | 137 | ||
135 | private final InternalAppsListener appsListener = new InternalAppsListener(); | 138 | private final InternalAppsListener appsListener = new InternalAppsListener(); |
139 | + private final NextAppToActivateValueListener nextAppToActivateListener = new NextAppToActivateValueListener(); | ||
136 | 140 | ||
137 | private Consumer<Status> statusChangeListener; | 141 | private Consumer<Status> statusChangeListener; |
138 | 142 | ||
... | @@ -168,6 +172,14 @@ public class DistributedApplicationStore extends ApplicationArchive | ... | @@ -168,6 +172,14 @@ public class DistributedApplicationStore extends ApplicationArchive |
168 | InternalState.class)) | 172 | InternalState.class)) |
169 | .build(); | 173 | .build(); |
170 | 174 | ||
175 | + nextAppToActivate = storageService.<Application>atomicValueBuilder() | ||
176 | + .withName("onos-apps-activation-order-value") | ||
177 | + .withSerializer(Serializer.using(KryoNamespaces.API)) | ||
178 | + .build() | ||
179 | + .asAtomicValue(); | ||
180 | + | ||
181 | + nextAppToActivate.addListener(nextAppToActivateListener); | ||
182 | + | ||
171 | executor = newSingleThreadScheduledExecutor(groupedThreads("onos/app", "store", log)); | 183 | executor = newSingleThreadScheduledExecutor(groupedThreads("onos/app", "store", log)); |
172 | statusChangeListener = status -> { | 184 | statusChangeListener = status -> { |
173 | if (status == Status.ACTIVE) { | 185 | if (status == Status.ACTIVE) { |
... | @@ -248,6 +260,7 @@ public class DistributedApplicationStore extends ApplicationArchive | ... | @@ -248,6 +260,7 @@ public class DistributedApplicationStore extends ApplicationArchive |
248 | clusterCommunicator.removeSubscriber(APP_BITS_REQUEST); | 260 | clusterCommunicator.removeSubscriber(APP_BITS_REQUEST); |
249 | apps.removeStatusChangeListener(statusChangeListener); | 261 | apps.removeStatusChangeListener(statusChangeListener); |
250 | apps.removeListener(appsListener); | 262 | apps.removeListener(appsListener); |
263 | + nextAppToActivate.removeListener(nextAppToActivateListener); | ||
251 | messageHandlingExecutor.shutdown(); | 264 | messageHandlingExecutor.shutdown(); |
252 | executor.shutdown(); | 265 | executor.shutdown(); |
253 | log.info("Stopped"); | 266 | log.info("Stopped"); |
... | @@ -346,15 +359,10 @@ public class DistributedApplicationStore extends ApplicationArchive | ... | @@ -346,15 +359,10 @@ public class DistributedApplicationStore extends ApplicationArchive |
346 | } | 359 | } |
347 | activateRequiredApps(vAppHolder.value().app()); | 360 | activateRequiredApps(vAppHolder.value().app()); |
348 | 361 | ||
349 | - // FIXME: Take a breath before the post-order operation to allow required app | ||
350 | - // activation events to fully propagate. There appears to be an out-of-order | ||
351 | - // event delivery issue that needs to be fixed. | ||
352 | - Tools.delay(FIXME_ACTIVATION_DELAY); | ||
353 | - | ||
354 | apps.computeIf(appId, v -> v != null && v.state() != ACTIVATED, | 362 | apps.computeIf(appId, v -> v != null && v.state() != ACTIVATED, |
355 | (k, v) -> new InternalApplicationHolder( | 363 | (k, v) -> new InternalApplicationHolder( |
356 | v.app(), ACTIVATED, v.permissions())); | 364 | v.app(), ACTIVATED, v.permissions())); |
357 | - | 365 | + nextAppToActivate.set(vAppHolder.value().app()); |
358 | } | 366 | } |
359 | } | 367 | } |
360 | 368 | ||
... | @@ -427,6 +435,20 @@ public class DistributedApplicationStore extends ApplicationArchive | ... | @@ -427,6 +435,20 @@ public class DistributedApplicationStore extends ApplicationArchive |
427 | } | 435 | } |
428 | } | 436 | } |
429 | 437 | ||
438 | + private class NextAppToActivateValueListener implements AtomicValueEventListener<Application> { | ||
439 | + | ||
440 | + @Override | ||
441 | + public void event(AtomicValueEvent<Application> event) { | ||
442 | + messageHandlingExecutor.execute(() -> { | ||
443 | + Application app = event.newValue(); | ||
444 | + String appName = app.id().name(); | ||
445 | + installAppIfNeeded(app); | ||
446 | + setActive(appName); | ||
447 | + delegate.notify(new ApplicationEvent(APP_ACTIVATED, app)); | ||
448 | + }); | ||
449 | + } | ||
450 | + } | ||
451 | + | ||
430 | /** | 452 | /** |
431 | * Listener to application state distributed map changes. | 453 | * Listener to application state distributed map changes. |
432 | */ | 454 | */ |
... | @@ -454,13 +476,10 @@ public class DistributedApplicationStore extends ApplicationArchive | ... | @@ -454,13 +476,10 @@ public class DistributedApplicationStore extends ApplicationArchive |
454 | } | 476 | } |
455 | 477 | ||
456 | private void setupApplicationAndNotify(ApplicationId appId, Application app, InternalState state) { | 478 | private void setupApplicationAndNotify(ApplicationId appId, Application app, InternalState state) { |
479 | + // ACTIVATED state is handled separately in NextAppToActivateValueListener | ||
457 | if (state == INSTALLED) { | 480 | if (state == INSTALLED) { |
458 | fetchBitsIfNeeded(app); | 481 | fetchBitsIfNeeded(app); |
459 | delegate.notify(new ApplicationEvent(APP_INSTALLED, app)); | 482 | delegate.notify(new ApplicationEvent(APP_INSTALLED, app)); |
460 | - } else if (state == ACTIVATED) { | ||
461 | - installAppIfNeeded(app); | ||
462 | - setActive(appId.name()); | ||
463 | - delegate.notify(new ApplicationEvent(APP_ACTIVATED, app)); | ||
464 | } else if (state == DEACTIVATED) { | 483 | } else if (state == DEACTIVATED) { |
465 | clearActive(appId.name()); | 484 | clearActive(appId.name()); |
466 | delegate.notify(new ApplicationEvent(APP_DEACTIVATED, app)); | 485 | delegate.notify(new ApplicationEvent(APP_DEACTIVATED, app)); | ... | ... |
-
Please register or login to post a comment