Skip to content

Commit

Permalink
[grid] Container existence won't be checked.
Browse files Browse the repository at this point in the history
Checking for container existence does one more
request to the docker daemon, and in moments of
high usage, this can query the daemon for all
containers unnecessarily. Instead of that, we
handle the response from the docker endpoint
and we log it. Ideally, we are stopping a
container when the user said `driver.quit()`,
so for the user does not really matter if the
container died or not. Logs are enough to
investigate issues.
  • Loading branch information
diemol committed Nov 12, 2020
1 parent 7b8bd72 commit 1b23c91
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 80 deletions.
8 changes: 5 additions & 3 deletions java/server/src/org/openqa/selenium/docker/Container.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.openqa.selenium.internal.Require;

import java.time.Duration;
import java.util.logging.Level;
import java.util.logging.Logger;

public class Container {
Expand All @@ -46,10 +47,11 @@ public void start() {
public void stop(Duration timeout) {
Require.nonNull("Timeout to wait for", timeout);

if (protocol.exists(id)) {
LOG.info("Stopping " + getId());

LOG.info("Stopping " + getId());
try {
protocol.stopContainer(id, timeout);
} catch (RuntimeException e) {
LOG.log(Level.WARNING, "Unable to stop container: " + e.getMessage(), e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,5 @@ public interface DockerProtocol {
Container create(ContainerConfig info);
void startContainer(ContainerId id) throws DockerException;
void stopContainer(ContainerId id, Duration timeout) throws DockerException;
boolean exists(ContainerId id);
ContainerInfo inspectContainer(ContainerId id) throws DockerException;
}

This file was deleted.

11 changes: 0 additions & 11 deletions java/server/src/org/openqa/selenium/docker/v1_40/V140Docker.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public class V140Docker implements DockerProtocol {
private final CreateContainer createContainer;
private final StartContainer startContainer;
private final StopContainer stopContainer;
private final ContainerExists containerExists;
private final InspectContainer inspectContainer;

public V140Docker(HttpHandler client) {
Expand All @@ -51,7 +50,6 @@ public V140Docker(HttpHandler client) {
createContainer = new CreateContainer(this, client);
startContainer = new StartContainer(client);
stopContainer = new StopContainer(client);
containerExists = new ContainerExists(client);
inspectContainer = new InspectContainer(client);
}

Expand Down Expand Up @@ -102,15 +100,6 @@ public void startContainer(ContainerId id) throws DockerException {
startContainer.apply(id);
}

@Override
public boolean exists(ContainerId id) {
Require.nonNull("Container id", id);

LOG.fine(String.format("Checking whether %s is running", id));

return containerExists.apply(id);
}

@Override
public void stopContainer(ContainerId id, Duration timeout) throws DockerException {
Require.nonNull("Container id", id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
attributeMap.put(AttributeKey.LOGGER_CLASS.getKey(),
EventAttribute.setValue(this.getClass().getName()));
LOG.info("Creating container, mapping container port 4444 to " + port);
Map<String, String> browserContainerEnvVars = getBrowserContainerEnvVars(sessionRequest.getCapabilities());
Map<String, String> browserContainerEnvVars =
getBrowserContainerEnvVars(sessionRequest.getCapabilities());
Container container = docker.create(
image(browserImage)
.env(browserContainerEnvVars)
Expand All @@ -156,7 +157,9 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {

EXCEPTION.accept(attributeMap, e);
attributeMap.put(AttributeKey.EXCEPTION_MESSAGE.getKey(),
EventAttribute.setValue("Unable to connect to docker server. Stopping container: " + e.getMessage()));
EventAttribute.setValue(
"Unable to connect to docker server. Stopping container: " +
e.getMessage()));
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);

container.stop(Duration.ofMinutes(1));
Expand Down

0 comments on commit 1b23c91

Please sign in to comment.