Skip to content

Commit a069dac

Browse files
committed
fix(fss): exclude waiting for non-autostartable services and their hard dependencies
fix(fss): exclude waiting for non-autostartable services and their hard dependencies
1 parent 74c2796 commit a069dac

File tree

5 files changed

+115
-54
lines changed

5 files changed

+115
-54
lines changed

src/main/java/com/aws/greengrass/deployment/KernelUpdateDeploymentTask.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.nio.file.Path;
3030
import java.util.Collections;
3131
import java.util.List;
32+
import java.util.Set;
3233
import java.util.concurrent.CancellationException;
3334
import java.util.concurrent.CompletableFuture;
3435
import java.util.concurrent.ExecutionException;
@@ -89,9 +90,7 @@ private void waitForServicesToStart() {
8990
Deployment.DeploymentStage stage = deployment.getDeploymentStage();
9091
DeploymentResult result = null;
9192
try {
92-
List<GreengrassService> servicesToTrack =
93-
kernel.orderedDependencies().stream().filter(GreengrassService::shouldAutoStart)
94-
.filter(o -> !kernel.getMain().equals(o)).collect(Collectors.toList());
93+
Set<GreengrassService> servicesToTrack = kernel.findAutoStartableServicesToTrack();
9594
long mergeTimestamp = kernel.getConfig().lookup("system", "rootpath").getModtime();
9695

9796
logger.atInfo().kv("serviceToTrack", servicesToTrack).kv("mergeTime", mergeTimestamp)

src/main/java/com/aws/greengrass/deployment/activator/DefaultActivator.java

Lines changed: 9 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,9 @@
1313
import com.aws.greengrass.lifecyclemanager.GreengrassService;
1414
import com.aws.greengrass.lifecyclemanager.Kernel;
1515
import com.aws.greengrass.lifecyclemanager.exceptions.ServiceLoadException;
16-
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1716

1817
import java.util.HashMap;
19-
import java.util.LinkedList;
2018
import java.util.Map;
21-
import java.util.Queue;
2219
import java.util.Set;
2320
import java.util.concurrent.CompletableFuture;
2421
import java.util.stream.Collectors;
@@ -82,6 +79,9 @@ public void activate(Map<String, Object> newConfig, Deployment deployment,
8279

8380
try {
8481
Set<GreengrassService> servicesToTrack = servicesChangeManager.servicesToTrack();
82+
// Exclude all non-autoStartable services and their dependers
83+
// Find the services which both changed and auto startable
84+
servicesToTrack.retainAll(kernel.findAutoStartableServicesToTrack());
8585
logger.atDebug(MERGE_CONFIG_EVENT_KEY).kv("serviceToTrack", servicesToTrack).kv("mergeTime", mergeTime)
8686
.log("Applied new service config. Waiting for services to complete update");
8787
waitForServicesToStart(servicesToTrack, mergeTime, kernel, totallyCompleteFuture);
@@ -144,10 +144,13 @@ void rollback(DeploymentDocument deploymentDocument, CompletableFuture<Deploymen
144144
// be expected to still be broken
145145
Set<GreengrassService> servicesToTrackForRollback = rollbackManager.servicesToTrack();
146146

147-
// Also don't track services if they have (transitive) hard dependencies on already-broken services
148-
Set<GreengrassService> brokenServiceAndDependers
149-
= findServiceDependers(servicesToTrackForRollback, rollbackManager.getAlreadyBrokenServices());
147+
// Convert broken services names from String to GreengrassService type
148+
Set<GreengrassService> brokenServices = servicesToTrackForRollback.stream()
149+
.filter(service -> rollbackManager.getAlreadyBrokenServices().contains(service.getName()))
150+
.collect(Collectors.toSet());
150151

152+
// Also don't track services if they have (transitive) hard dependencies on already-broken services
153+
Set<GreengrassService> brokenServiceAndDependers = kernel.findDependers(brokenServices);
151154
servicesToTrackForRollback.removeAll(brokenServiceAndDependers);
152155

153156
logger.atInfo(MERGE_CONFIG_EVENT_KEY)
@@ -170,37 +173,6 @@ void rollback(DeploymentDocument deploymentDocument, CompletableFuture<Deploymen
170173
}
171174
}
172175

173-
/**
174-
* Finds all services which are dependers of given broken services, directly or indirectly
175-
* This method performs a breadth-first search, starting from the broken services and traversing through
176-
* service dependencies.
177-
* @param rollbackServices the set of rollback services to track
178-
* @param brokenServiceNames the set of broken service names
179-
* @return a set of all services depending on the broken services, including themselves
180-
*/
181-
@SuppressFBWarnings("RV_RETURN_VALUE_IGNORED_BAD_PRACTICE")
182-
private Set<GreengrassService> findServiceDependers(final Set<GreengrassService> rollbackServices,
183-
final Set<String> brokenServiceNames) {
184-
185-
Set<GreengrassService> dependerServices = rollbackServices.stream()
186-
.filter(service -> brokenServiceNames.contains(service.getName()))
187-
.collect(Collectors.toSet());
188-
Queue<GreengrassService> dependers = new LinkedList<>(dependerServices);
189-
190-
// Breadth-first search to find all dependent services, staring from broken services
191-
while (!dependers.isEmpty()) {
192-
GreengrassService currentService = dependers.poll();
193-
for (GreengrassService depender : currentService.getHardDependers()) {
194-
// Ensure dependers haven't been processed
195-
if (dependerServices.add(depender)) {
196-
dependers.offer(depender);
197-
}
198-
}
199-
}
200-
return dependerServices;
201-
}
202-
203-
204176
private void handleFailureRollback(CompletableFuture totallyCompleteFuture, Throwable deploymentFailureCause,
205177
Throwable rollbackFailureCause) {
206178
// Rollback execution failed
@@ -209,5 +181,4 @@ private void handleFailureRollback(CompletableFuture totallyCompleteFuture, Thro
209181
totallyCompleteFuture.complete(new DeploymentResult(DeploymentResult.DeploymentStatus.FAILED_UNABLE_TO_ROLLBACK,
210182
deploymentFailureCause));
211183
}
212-
213184
}

src/main/java/com/aws/greengrass/lifecyclemanager/Kernel.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,13 @@
8080
import java.util.HashMap;
8181
import java.util.HashSet;
8282
import java.util.LinkedHashSet;
83+
import java.util.LinkedList;
8384
import java.util.List;
8485
import java.util.Locale;
8586
import java.util.Map;
8687
import java.util.Optional;
88+
import java.util.Queue;
89+
import java.util.Set;
8790
import java.util.concurrent.ConcurrentHashMap;
8891
import java.util.concurrent.Executor;
8992
import java.util.concurrent.ExecutorService;
@@ -913,4 +916,49 @@ private void setupProxy() {
913916
public List<String> getSupportedCapabilities() {
914917
return SUPPORTED_CAPABILITIES;
915918
}
919+
920+
/**
921+
* Finds all auto startable services with auto startable dependencies.
922+
* This method performs a breadth-first search, starting from the target services and traversing through
923+
* all hard dependencies and exclude non auto startable services from.
924+
*
925+
* @return a set of all services that only contains auto startable services and their dependencies are all
926+
* auto startable services
927+
*/
928+
public Set<GreengrassService> findAutoStartableServicesToTrack() {
929+
// Find all non auto startable services
930+
Set<GreengrassService> nonAutoStartableServices = orderedDependencies().stream()
931+
.filter(service -> !service.shouldAutoStart()).collect(Collectors.toSet());
932+
933+
Set<GreengrassService> nonAutoStartableDependers = findDependers(nonAutoStartableServices);
934+
935+
// Return the set which excludes all non auto startable services and their dependers
936+
return orderedDependencies().stream()
937+
.filter(service -> !nonAutoStartableDependers.contains(service))
938+
.collect(Collectors.toSet());
939+
}
940+
941+
/**
942+
* Finds all services which are dependers of initial services, directly or indirectly
943+
* This method performs a breadth-first search, starting from the initial services and traversing through
944+
* all hard dependencies.
945+
* @param initialServices the set of services that we want to find dependers
946+
* @return a set of all services that depend on the target services, including the initial services
947+
*/
948+
@SuppressFBWarnings("RV_RETURN_VALUE_IGNORED_BAD_PRACTICE")
949+
public Set<GreengrassService> findDependers(Set<GreengrassService> initialServices) {
950+
Queue<GreengrassService> dependers = new LinkedList<>(initialServices);
951+
952+
// Breadth-first search to find all dependent services, staring from non auto startable services
953+
while (!dependers.isEmpty()) {
954+
GreengrassService currentService = dependers.poll();
955+
for (GreengrassService depender : currentService.getHardDependers()) {
956+
// Ensure dependers haven't been processed
957+
if (initialServices.add(depender)) {
958+
dependers.offer(depender);
959+
}
960+
}
961+
}
962+
return initialServices;
963+
}
916964
}

src/main/java/com/aws/greengrass/lifecyclemanager/KernelLifecycle.java

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -600,19 +600,19 @@ GreengrassService getMain() {
600600
* @return true if all services in terminal states
601601
*/
602602
public boolean allServicesInTerminalState() {
603-
List<GreengrassService> servicesToTrack = kernel.orderedDependencies().stream()
604-
.filter(GreengrassService::shouldAutoStart).collect(Collectors.toList());
603+
List<GreengrassService> servicesToTrack = kernel.findAutoStartableServicesToTrack()
604+
.stream().collect(Collectors.toList());
605605
return servicesToTrack.stream().allMatch(service -> {
606-
State state = service.getState();
607-
// service is broken
608-
if (State.BROKEN.equals(state)) {
609-
return true;
610-
}
611-
// or service has reached desired state, and it is either running or finished
612-
if (service.reachedDesiredState()) {
613-
return State.RUNNING.equals(state) || State.FINISHED.equals(state);
614-
}
615-
return false;
616-
});
606+
State state = service.getState();
607+
// service is broken
608+
if (State.BROKEN.equals(state)) {
609+
return true;
610+
}
611+
// or service has reached desired state, and it is either running or finished
612+
if (service.reachedDesiredState()) {
613+
return State.RUNNING.equals(state) || State.FINISHED.equals(state);
614+
}
615+
return false;
616+
});
617617
}
618618
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package com.aws.greengrass.util;
7+
8+
import com.aws.greengrass.lifecyclemanager.GreengrassService;
9+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
10+
11+
import java.util.LinkedList;
12+
import java.util.Queue;
13+
import java.util.Set;
14+
15+
public final class DependerFinder {
16+
private DependerFinder() {
17+
}
18+
19+
/**
20+
* Finds all services which are dependers of target services, directly or indirectly
21+
* This method performs a breadth-first search, starting from the target services and traversing through
22+
* all hard dependencies.
23+
* @param targetServices the set of services that we want to find their dependers
24+
* @return a set of all services that depend on the target services, including the target services
25+
*/
26+
@SuppressFBWarnings("RV_RETURN_VALUE_IGNORED_BAD_PRACTICE")
27+
public static Set<GreengrassService> findTargetServicesDependers(final Set<GreengrassService> targetServices) {
28+
Queue<GreengrassService> dependers = new LinkedList<>(targetServices);
29+
30+
// Breadth-first search to find all dependent services, staring from broken services
31+
while (!dependers.isEmpty()) {
32+
GreengrassService currentService = dependers.poll();
33+
for (GreengrassService depender : currentService.getHardDependers()) {
34+
// Ensure dependers haven't been processed
35+
if (targetServices.add(depender)) {
36+
dependers.offer(depender);
37+
}
38+
}
39+
}
40+
return targetServices;
41+
}
42+
43+
}

0 commit comments

Comments
 (0)