Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CODENVY-1403: fix bug in machine removal #4154

Merged
merged 3 commits into from
Feb 16, 2017
Merged

CODENVY-1403: fix bug in machine removal #4154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
18e2440
CODENVY-1403: fix bug in machine removal
Feb 16, 2017
a42a7be
fixup! CODENVY-1403: fix bug in machine removal
Feb 16, 2017
f811b57
fixup! fixup! CODENVY-1403: fix bug in machine removal Signed-off-by:…
Feb 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ...in-docker-machine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstance.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,7 @@ private String generateRepository() {
public void destroy() throws MachineException {
try {
outputConsumer.close();
} catch (IOException ignored) {
}
} catch (IOException ignored) {}

machineProcesses.clear();
processesCleaner.untrackProcesses(getId());
Expand Down
114 changes: 73 additions & 41 deletions ...-workspace/src/main/java/org/eclipse/che/api/environment/server/CheEnvironmentEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
package org.eclipse.che.api.environment.server;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;

import org.eclipse.che.api.agent.server.AgentRegistry;
import org.eclipse.che.api.agent.server.exception.AgentException;
Expand Down Expand Up @@ -59,6 +58,7 @@
import org.eclipse.che.api.machine.server.spi.SnapshotDao;
import org.eclipse.che.api.machine.server.util.RecipeDownloader;
import org.eclipse.che.api.machine.shared.dto.event.MachineStatusEvent;
import org.eclipse.che.api.workspace.server.WorkspaceSharedPool;
import org.eclipse.che.api.workspace.server.model.impl.EnvironmentImpl;
import org.eclipse.che.api.workspace.server.model.impl.ExtendedMachineImpl;
import org.eclipse.che.commons.annotation.Nullable;
Expand Down Expand Up @@ -88,6 +88,7 @@
import java.util.regex.Pattern;

import static java.lang.String.format;
import static java.util.Collections.emptyList;
import static java.util.stream.Collectors.toList;
import static org.eclipse.che.api.machine.server.event.InstanceStateEvent.Type.DIE;
import static org.eclipse.che.api.machine.server.event.InstanceStateEvent.Type.OOM;
Expand Down Expand Up @@ -122,6 +123,7 @@ public class CheEnvironmentEngine {
private final Pattern recipeApiPattern;
private final ContainerNameGenerator containerNameGenerator;
private final AgentRegistry agentRegistry;
private final WorkspaceSharedPool sharedPool;

private volatile boolean isPreDestroyInvoked;

Expand All @@ -138,7 +140,8 @@ public CheEnvironmentEngine(SnapshotDao snapshotDao,
@Named("che.api") String apiEndpoint,
RecipeDownloader recipeDownloader,
ContainerNameGenerator containerNameGenerator,
AgentRegistry agentRegistry) {
AgentRegistry agentRegistry,
WorkspaceSharedPool sharedPool) {
this.snapshotDao = snapshotDao;
this.eventService = eventService;
this.environmentParser = environmentParser;
Expand All @@ -147,6 +150,7 @@ public CheEnvironmentEngine(SnapshotDao snapshotDao,
this.infrastructureProvisioner = infrastructureProvisioner;
this.recipeDownloader = recipeDownloader;
this.agentRegistry = agentRegistry;
this.sharedPool = sharedPool;
this.environments = new ConcurrentHashMap<>();
this.machineInstanceProviders = machineInstanceProviders;
this.machineLogsDir = new File(machineLogsDir);
Expand All @@ -157,8 +161,6 @@ public CheEnvironmentEngine(SnapshotDao snapshotDao,
apiEndpoint.substring(apiEndpoint.indexOf(":")) +
"/recipe/.*$)|(^/recipe/.*$)");
this.containerNameGenerator = containerNameGenerator;

eventService.subscribe(new MachineCleaner());
}

/**
Expand Down Expand Up @@ -308,7 +310,7 @@ public List<Instance> start(String workspaceId,
*/
public void stop(String workspaceId) throws EnvironmentNotRunningException,
ServerException {
List<Instance> machinesCopy = null;
List<Instance> machinesCopy;
EnvironmentHolder environmentHolder;
try (@SuppressWarnings("unused") Unlocker u = stripedLocks.readLock(workspaceId)) {
environmentHolder = environments.get(workspaceId);
Expand All @@ -320,13 +322,13 @@ public void stop(String workspaceId) throws EnvironmentNotRunningException,
List<Instance> machines = environmentHolder.machines;
if (machines != null && !machines.isEmpty()) {
machinesCopy = new ArrayList<>(machines);
} else {
machinesCopy = emptyList();
}
}

// long operation - perform out of lock
if (machinesCopy != null) {
destroyEnvironment(environmentHolder.networkId, machinesCopy);
}
destroyEnvironment(environmentHolder.networkId, machinesCopy);

try (@SuppressWarnings("unused") Unlocker u = stripedLocks.writeLock(workspaceId)) {
environments.remove(workspaceId);
Expand Down Expand Up @@ -1180,11 +1182,14 @@ private void destroyMachine(Instance machine) throws MachineException {
}

@SuppressWarnings("unused")
@VisibleForTesting
@PostConstruct
private void createLogsDir() {
void init() {
if (!(machineLogsDir.exists() || machineLogsDir.mkdirs())) {
throw new IllegalStateException("Unable create directory " + machineLogsDir.getAbsolutePath());
}

eventService.subscribe(new MachineCleaner());
}

/**
Expand Down Expand Up @@ -1279,6 +1284,26 @@ private CheServiceImpl machineConfigToService(MachineConfig machineConfig) throw
return service;
}

// Removes machine from environment but doesn't stop it.
@VisibleForTesting
@Nullable
Instance removeMachineFromEnvironment(String workspaceId, String machineId) {
try (@SuppressWarnings("unused") Unlocker u = stripedLocks.writeLock(workspaceId)) {
EnvironmentHolder environmentHolder = environments.get(workspaceId);
if (environmentHolder == null || environmentHolder.status != EnvStatus.RUNNING) {
// should not happen
return null;
}
for (Instance machine : environmentHolder.machines) {
if (machine.getId().equals(machineId)) {
environmentHolder.machines.remove(machine);
return machine;
}
}
return null;
}
}

private enum EnvStatus {
STARTING,
RUNNING,
Expand Down Expand Up @@ -1337,39 +1362,46 @@ public int hashCode() {
private class MachineCleaner implements EventSubscriber<InstanceStateEvent> {
@Override
public void onEvent(InstanceStateEvent event) {
if ((event.getType() == OOM) || (event.getType() == DIE)) {
EnvironmentHolder environmentHolder;
try (@SuppressWarnings("unused") Unlocker u = stripedLocks.readLock("workspaceId")) {
environmentHolder = environments.get(event.getWorkspaceId());
}
if (environmentHolder != null) {
for (Instance instance : environmentHolder.machines) {
if (instance.getId().equals(event.getMachineId())) {
String message = "Machine is destroyed. ";
if (event.getType() == OOM) {
message = message +
"The processes in this machine need more RAM. This machine started with " +
instance.getConfig().getLimits().getRam() +
"MB. Create a new machine configuration that allocates additional RAM or increase " +
"the workspace RAM limit in the user dashboard.";
}

try {
if (!Strings.isNullOrEmpty(message)) {
instance.getLogger().writeLine(message);
}
} catch (IOException ignore) {
}

eventService.publish(newDto(MachineStatusEvent.class)
.withEventType(MachineStatusEvent.EventType.DESTROYED)
.withDev(instance.getConfig().isDev())
.withMachineId(instance.getId())
.withWorkspaceId(instance.getWorkspaceId())
.withMachineName(instance.getConfig().getName()));
}
String machineId = event.getMachineId();
String workspaceId = event.getWorkspaceId();
InstanceStateEvent.Type eventType = event.getType();
// cleanup machine if event about instance failure comes
if ((eventType == OOM) || (eventType == DIE)) {
sharedPool.execute(() -> {
Instance instance = removeMachineFromEnvironment(workspaceId, machineId);
if (instance == null) {
// should not happen
return;
}
}

String message = "Machine is destroyed";
if (eventType == OOM) {
message = message +
". The processes in this machine need more RAM. This machine started with " +
instance.getConfig().getLimits().getRam() +
"MB. Create a new machine configuration that allocates additional RAM or increase " +
"the workspace RAM limit in the user dashboard.";
}
MachineStatusEvent destroyedEvent = newDto(MachineStatusEvent.class)
.withEventType(MachineStatusEvent.EventType.DESTROYED)
.withDev(instance.getConfig().isDev())
.withMachineId(machineId)
.withWorkspaceId(workspaceId)
.withMachineName(instance.getConfig().getName())
.withError(message);

try {
instance.getLogger().writeLine(message);
} catch (IOException ignore) {}

try {
instance.destroy();
} catch (MachineException e) {
LOG.warn("Destroying of machine {} in workspace {} where container was unexpectedly stopped failed. Error: {}, {}",
machineId, workspaceId, e.getLocalizedMessage());
}
eventService.publish(destroyedEvent);
});
}
}
}
Expand Down
Loading