From 59b73bc2840a6ed652acc6c0d7e79b6a18a3089e Mon Sep 17 00:00:00 2001 From: Emmanuel Stanley Date: Thu, 17 Oct 2024 12:15:15 +0100 Subject: [PATCH 1/6] feat: Implement pre-initialized Docker container pool to improve /eval request performance - Added a pool of pre-initialized Docker containers, each assigned a unique session ID and ready for immediate use. - Pre-configured each container with the necessary startup scripts to ensure environments are ready for /eval requests. - On receiving a new /eval request, the system now allocates a container from the pool, reducing the need to create and initialize a container on demand. - Improved request latency by significantly reducing the time taken to start and initialize Docker containers during each session. --- .../jshellapi/dto/ContainerState.java | 9 + .../jshellapi/service/DockerService.java | 242 ++++++++++++++---- .../jshellapi/service/JShellService.java | 49 ++-- .../service/JShellSessionService.java | 8 +- .../togetherjava/jshellapi/service/Utils.java | 8 + 5 files changed, 226 insertions(+), 90 deletions(-) create mode 100644 JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java new file mode 100644 index 0000000..181d7af --- /dev/null +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java @@ -0,0 +1,9 @@ +package org.togetherjava.jshellapi.dto; + +import java.io.BufferedReader; +import java.io.BufferedWriter; +import java.io.InputStream; +import java.io.OutputStream; + +public record ContainerState(boolean isCached, String containerId, BufferedReader containerOutput, BufferedWriter containerInput) { +} diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java index 3490aef..a3357ac 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java @@ -2,6 +2,7 @@ import com.github.dockerjava.api.DockerClient; import com.github.dockerjava.api.async.ResultCallback; +import com.github.dockerjava.api.command.InspectContainerResponse; import com.github.dockerjava.api.command.PullImageResultCallback; import com.github.dockerjava.api.model.*; import com.github.dockerjava.core.DefaultDockerClientConfig; @@ -10,28 +11,35 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.DisposableBean; -import org.springframework.lang.Nullable; import org.springframework.stereotype.Service; import org.togetherjava.jshellapi.Config; +import org.togetherjava.jshellapi.dto.ContainerState; import java.io.*; import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.*; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.*; @Service public class DockerService implements DisposableBean { private static final Logger LOGGER = LoggerFactory.getLogger(DockerService.class); private static final String WORKER_LABEL = "jshell-api-worker"; private static final UUID WORKER_UNIQUE_ID = UUID.randomUUID(); + private static final String IMAGE_NAME = "togetherjava.org:5001/togetherjava/jshellwrapper"; + private static final String IMAGE_TAG = "master"; private final DockerClient client; + private final Config config; + private final ExecutorService executor = Executors.newSingleThreadExecutor(); + private final ConcurrentHashMap cachedContainers = new ConcurrentHashMap<>(); + private final StartupScriptsService startupScriptsService; private final String jshellWrapperBaseImageName; - public DockerService(Config config) { + public DockerService(Config config, StartupScriptsService startupScriptsService) throws InterruptedException, IOException { + this.startupScriptsService = startupScriptsService; DefaultDockerClientConfig clientConfig = DefaultDockerClientConfig.createDefaultConfigBuilder().build(); ApacheDockerHttpClient httpClient = @@ -41,11 +49,16 @@ public DockerService(Config config) { .connectionTimeout(Duration.ofSeconds(config.dockerConnectionTimeout())) .build(); this.client = DockerClientImpl.getInstance(clientConfig, httpClient); + this.config = config; this.jshellWrapperBaseImageName = config.jshellWrapperImageName().split(Config.JSHELL_WRAPPER_IMAGE_NAME_TAG)[0]; + if (!isImagePresentLocally()) { + pullImage(); + } cleanupLeftovers(WORKER_UNIQUE_ID); + executor.submit(() -> initializeCachedContainer(StartupScriptId.EMPTY)); } private void cleanupLeftovers(UUID currentId) { @@ -62,80 +75,198 @@ private void cleanupLeftovers(UUID currentId) { } } - public String spawnContainer(long maxMemoryMegs, long cpus, @Nullable String cpuSetCpus, - String name, Duration evalTimeout, long sysoutLimit) throws InterruptedException { - - boolean presentLocally = client.listImagesCmd() - .withFilter("reference", List.of(jshellWrapperBaseImageName)) - .exec() - .stream() - .flatMap(it -> Arrays.stream(it.getRepoTags())) - .anyMatch(it -> it.endsWith(Config.JSHELL_WRAPPER_IMAGE_NAME_TAG)); + /** + * Checks if the Docker image with the given name and tag is present locally. + * + * @return true if the image is present, false otherwise. + */ + private boolean isImagePresentLocally() { + return client.listImagesCmd() + .withFilter("reference", List.of(jshellWrapperBaseImageName)) + .exec() + .stream() + .flatMap(it -> Arrays.stream(it.getRepoTags())) + .anyMatch(it -> it.endsWith(Config.JSHELL_WRAPPER_IMAGE_NAME_TAG)); + } - if (!presentLocally) { + /** + * Pulls the Docker image. + */ + private void pullImage() throws InterruptedException { + if (!isImagePresentLocally()) { client.pullImageCmd(jshellWrapperBaseImageName) - .withTag("master") - .exec(new PullImageResultCallback()) - .awaitCompletion(5, TimeUnit.MINUTES); + .withTag(IMAGE_TAG) + .exec(new PullImageResultCallback()) + .awaitCompletion(5, TimeUnit.MINUTES); } + } - return client - .createContainerCmd(jshellWrapperBaseImageName + Config.JSHELL_WRAPPER_IMAGE_NAME_TAG) - .withHostConfig(HostConfig.newHostConfig() + /** + * Creates a Docker container with the given name. + * + * @param name The name of the container to create. + * @return The ID of the created container. + */ + public String createContainer(String name) { + HostConfig hostConfig = HostConfig.newHostConfig() .withAutoRemove(true) .withInit(true) .withCapDrop(Capability.ALL) .withNetworkMode("none") .withPidsLimit(2000L) .withReadonlyRootfs(true) - .withMemory(maxMemoryMegs * 1024 * 1024) - .withCpuCount(cpus) - .withCpusetCpus(cpuSetCpus)) - .withStdinOpen(true) - .withAttachStdin(true) - .withAttachStderr(true) - .withAttachStdout(true) - .withEnv("evalTimeoutSeconds=" + evalTimeout.toSeconds(), - "sysOutCharLimit=" + sysoutLimit) - .withLabels(Map.of(WORKER_LABEL, WORKER_UNIQUE_ID.toString())) - .withName(name) - .exec() - .getId(); + .withMemory((long) config.dockerMaxRamMegaBytes() * 1024 * 1024) + .withCpuCount((long) Math.ceil(config.dockerCPUsUsage())) + .withCpusetCpus(config.dockerCPUSetCPUs()); + + return client.createContainerCmd(jshellWrapperBaseImageName + Config.JSHELL_WRAPPER_IMAGE_NAME_TAG) + .withHostConfig(hostConfig) + .withStdinOpen(true) + .withAttachStdin(true) + .withAttachStderr(true) + .withAttachStdout(true) + .withEnv("evalTimeoutSeconds=" + config.evalTimeoutSeconds(), + "sysOutCharLimit=" + config.sysOutCharLimit()) + .withLabels(Map.of(WORKER_LABEL, WORKER_UNIQUE_ID.toString())) + .withName(name) + .exec() + .getId(); + } + + /** + * Spawns a new Docker container with specified configurations. + * + * @param name Name of the container. + * @param startupScriptId Script to initialize the container with. + * @return The ContainerState of the newly created container. + */ + public ContainerState initializeContainer(String name, StartupScriptId startupScriptId) throws IOException { + if (cachedContainers.isEmpty() || !cachedContainers.containsKey(startupScriptId)) { + String containerId = createContainer(name); + return setupContainerWithScript(containerId, true, startupScriptId); + } + String containerId = cachedContainers.get(startupScriptId); + executor.submit(() -> initializeCachedContainer(startupScriptId)); + // Rename container with new name. + client.renameContainerCmd(containerId).withName(name).exec(); + return setupContainerWithScript(containerId, false, startupScriptId); + } + + /** + * Initializes a new cached docker container with specified configurations. + * + * @param startupScriptId Script to initialize the container with. + */ + private void initializeCachedContainer(StartupScriptId startupScriptId) { + String containerName = cachedContainerName(); + String id = createContainer(containerName); + startContainer(id); + + try (PipedInputStream containerInput = new PipedInputStream(); + BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(new PipedOutputStream(containerInput)))) { + attachToContainer(id, containerInput); + + writer.write(Utils.sanitizeStartupScript(startupScriptsService.get(startupScriptId))); + writer.newLine(); + writer.flush(); + + cachedContainers.put(startupScriptId, id); + } catch (IOException e) { + killContainerByName(containerName); + throw new RuntimeException(e); + } } - public InputStream startAndAttachToContainer(String containerId, InputStream stdin) - throws IOException { + /** + * + * @param containerId The id of the container + * @param isCached Indicator if the container is cached or new + * @param startupScriptId The startup script id of the session + * @return ContainerState of the spawned container. + * @throws IOException if an I/O error occurs + */ + private ContainerState setupContainerWithScript(String containerId, boolean isCached, StartupScriptId startupScriptId) throws IOException { + if (!isCached) { + startContainer(containerId); + } + PipedInputStream containerInput = new PipedInputStream(); + BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(new PipedOutputStream(containerInput))); + + InputStream containerOutput = attachToContainer(containerId, containerInput); + BufferedReader reader = new BufferedReader(new InputStreamReader(containerOutput)); + + if (!isCached) { + writer.write(Utils.sanitizeStartupScript(startupScriptsService.get(startupScriptId))); + writer.newLine(); + writer.flush(); + } + + return new ContainerState(isCached, containerId, reader, writer); + } + + /** + * Creates a new container + * @param containerId the ID of the container to start + */ + public void startContainer(String containerId) { + if (!isContainerRunning(containerId)) { + client.startContainerCmd(containerId).exec(); + } + } + + /** + * Attaches to a running Docker container's input (stdin) and output streams (stdout, stderr). + * Logs any output from stderr and returns an InputStream to read stdout. + * + * @param containerId the ID of the running container to attach to + * @param containerInput the input stream (containerInput) to send to the container + * @return InputStream to read the container's stdout + * @throws IOException if an I/O error occurs + */ + public InputStream attachToContainer(String containerId, InputStream containerInput) throws IOException { PipedInputStream pipeIn = new PipedInputStream(); PipedOutputStream pipeOut = new PipedOutputStream(pipeIn); client.attachContainerCmd(containerId) - .withLogs(true) - .withFollowStream(true) - .withStdOut(true) - .withStdErr(true) - .withStdIn(stdin) - .exec(new ResultCallback.Adapter<>() { - @Override - public void onNext(Frame object) { - try { - String payloadString = - new String(object.getPayload(), StandardCharsets.UTF_8); - if (object.getStreamType() == StreamType.STDOUT) { - pipeOut.write(object.getPayload()); - } else { - LOGGER.warn("Received STDERR from container {}: {}", containerId, - payloadString); + .withLogs(true) + .withFollowStream(true) + .withStdOut(true) + .withStdErr(true) + .withStdIn(containerInput) + .exec(new ResultCallback.Adapter<>() { + @Override + public void onNext(Frame object) { + try { + String payloadString = new String(object.getPayload(), StandardCharsets.UTF_8); + if (object.getStreamType() == StreamType.STDOUT) { + pipeOut.write(object.getPayload()); // Write stdout data to pipeOut + } else { + LOGGER.warn("Received STDERR from container {}: {}", containerId, payloadString); + } + } catch (IOException e) { + throw new UncheckedIOException(e); } - } catch (IOException e) { - throw new UncheckedIOException(e); } - } - }); + }); - client.startContainerCmd(containerId).exec(); return pipeIn; } + /** + * Checks if the Docker container with the given ID is currently running. + * + * @param containerId the ID of the container to check + * @return true if the container is running, false otherwise + */ + public boolean isContainerRunning(String containerId) { + InspectContainerResponse containerResponse = client.inspectContainerCmd(containerId).exec(); + return Boolean.TRUE.equals(containerResponse.getState().getRunning()); + } + + private String cachedContainerName() { + return "cached_session_" + UUID.randomUUID(); + } + public void killContainerByName(String name) { LOGGER.debug("Fetching container to kill {}.", name); List containers = client.listContainersCmd().withNameFilter(Set.of(name)).exec(); @@ -156,6 +287,7 @@ public boolean isDead(String containerName) { @Override public void destroy() throws Exception { LOGGER.info("destroy() called. Destroying all containers..."); + executor.shutdown(); cleanupLeftovers(UUID.randomUUID()); client.close(); } diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellService.java index c0505c3..339b9e8 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellService.java @@ -4,6 +4,7 @@ import org.slf4j.LoggerFactory; import org.springframework.lang.Nullable; +import org.togetherjava.jshellapi.Config; import org.togetherjava.jshellapi.dto.*; import org.togetherjava.jshellapi.exceptions.DockerException; @@ -30,34 +31,30 @@ public class JShellService { private final DockerService dockerService; private final int startupScriptSize; - public JShellService(DockerService dockerService, JShellSessionService sessionService, - String id, long timeout, boolean renewable, long evalTimeout, - long evalTimeoutValidationLeeway, int sysOutCharLimit, int maxMemory, double cpus, - @Nullable String cpuSetCpus, String startupScript) throws DockerException { + public JShellService( + DockerService dockerService, + JShellSessionService sessionService, + SessionInfo sessionInfo, + Config config + ) throws DockerException { this.dockerService = dockerService; this.sessionService = sessionService; - this.id = id; - this.timeout = timeout; - this.renewable = renewable; - this.evalTimeout = evalTimeout; - this.evalTimeoutValidationLeeway = evalTimeoutValidationLeeway; + this.id = sessionInfo.id(); + this.timeout = config.dockerConnectionTimeout(); + this.renewable = sessionInfo.renewable(); + this.evalTimeout = sessionInfo.evalTimeout(); + this.evalTimeoutValidationLeeway = sessionInfo.evalTimeoutValidationLeeway(); this.lastTimeoutUpdate = Instant.now(); + if (!dockerService.isDead(containerName())) { LOGGER.warn("Tried to create an existing container {}.", containerName()); throw new DockerException("The session isn't completely destroyed, try again later."); } + try { - String containerId = dockerService.spawnContainer(maxMemory, (long) Math.ceil(cpus), - cpuSetCpus, containerName(), Duration.ofSeconds(evalTimeout), sysOutCharLimit); - PipedInputStream containerInput = new PipedInputStream(); - this.writer = new BufferedWriter( - new OutputStreamWriter(new PipedOutputStream(containerInput))); - InputStream containerOutput = - dockerService.startAndAttachToContainer(containerId, containerInput); - reader = new BufferedReader(new InputStreamReader(containerOutput)); - writer.write(sanitize(startupScript)); - writer.newLine(); - writer.flush(); + ContainerState containerState = dockerService.initializeContainer(containerName(), sessionInfo.startupScriptId()); + this.writer = containerState.containerInput(); + this.reader = containerState.containerOutput(); checkContainerOK(); startupScriptSize = Integer.parseInt(reader.readLine()); } catch (Exception e) { @@ -127,7 +124,7 @@ private JShellResult readResult() throws IOException, NumberFormatException, Doc int errorCount = Integer.parseInt(reader.readLine()); List errors = new ArrayList<>(); for (int i = 0; i < errorCount; i++) { - errors.add(desanitize(reader.readLine())); + errors.add(Utils.deSanitizeStartupScript((reader.readLine()))); } yield new JShellEvalAbortionCause.CompileTimeErrorAbortionCause(errors); } @@ -140,7 +137,7 @@ private JShellResult readResult() throws IOException, NumberFormatException, Doc abortion = new JShellEvalAbortion(causeSource, remainingSource, abortionCause); } boolean stdoutOverflow = Boolean.parseBoolean(reader.readLine()); - String stdout = desanitize(reader.readLine()); + String stdout = Utils.deSanitizeStartupScript(reader.readLine()); return new JShellResult(snippetResults, abortion, stdoutOverflow, stdout); } @@ -282,14 +279,6 @@ private void stopOperation() { doingOperation = false; } - private static String sanitize(String s) { - return s.replace("\\", "\\\\").replace("\n", "\\n"); - } - - private static String desanitize(String text) { - return text.replace("\\n", "\n").replace("\\\\", "\\"); - } - private static String cleanCode(String code) { return code.translateEscapes(); } diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellSessionService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellSessionService.java index 98cfd24..c189551 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellSessionService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellSessionService.java @@ -88,11 +88,9 @@ private synchronized JShellService createSession(SessionInfo sessionInfo) "Too many sessions, try again later :(."); } LOGGER.info("Creating session : {}.", sessionInfo); - JShellService service = new JShellService(dockerService, this, sessionInfo.id(), - sessionInfo.sessionTimeout(), sessionInfo.renewable(), sessionInfo.evalTimeout(), - sessionInfo.evalTimeoutValidationLeeway(), sessionInfo.sysOutCharLimit(), - config.dockerMaxRamMegaBytes(), config.dockerCPUsUsage(), config.dockerCPUSetCPUs(), - startupScriptsService.get(sessionInfo.startupScriptId())); + JShellService service = new JShellService(dockerService, this, sessionInfo, config); + + jshellSessions.put(sessionInfo.id(), service); return service; } diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/Utils.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/Utils.java index 0c204e5..dc16ba5 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/Utils.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/Utils.java @@ -28,4 +28,12 @@ public static > Stream predicate(Class c, Predicate p } return Arrays.stream(enumConstants).filter(predicate); } + + public static String sanitizeStartupScript(String s) { + return s.replace("\\", "\\\\").replace("\n", "\\n"); + } + + public static String deSanitizeStartupScript(String text) { + return text.replace("\\n", "\n").replace("\\\\", "\\"); + } } From 7d21142c0f60f32fb9b8ce361bcf057a9872ca4a Mon Sep 17 00:00:00 2001 From: Emmanuel Stanley Date: Thu, 17 Oct 2024 12:44:54 +0100 Subject: [PATCH 2/6] chore: Apply code style checks with Spotless and linting with SonarQube - Ran code formatting and styling checks using Spotless to ensure consistent code style. - Performed code analysis and linting with SonarQube to identify and address potential issues. - Fixed minor issues flagged during the linting process to enhance code quality. --- .../jshellapi/dto/ContainerState.java | 5 +- .../jshellapi/service/DockerService.java | 79 +++++++++++-------- .../jshellapi/service/JShellService.java | 13 +-- .../service/JShellSessionService.java | 1 - 4 files changed, 50 insertions(+), 48 deletions(-) diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java index 181d7af..695baf2 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java @@ -2,8 +2,7 @@ import java.io.BufferedReader; import java.io.BufferedWriter; -import java.io.InputStream; -import java.io.OutputStream; -public record ContainerState(boolean isCached, String containerId, BufferedReader containerOutput, BufferedWriter containerInput) { +public record ContainerState(boolean isCached, String containerId, BufferedReader containerOutput, + BufferedWriter containerInput) { } diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java index a3357ac..a48e656 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java @@ -33,7 +33,8 @@ public class DockerService implements DisposableBean { private final DockerClient client; private final Config config; private final ExecutorService executor = Executors.newSingleThreadExecutor(); - private final ConcurrentHashMap cachedContainers = new ConcurrentHashMap<>(); + private final ConcurrentHashMap cachedContainers = + new ConcurrentHashMap<>(); private final StartupScriptsService startupScriptsService; private final String jshellWrapperBaseImageName; @@ -109,15 +110,15 @@ private void pullImage() throws InterruptedException { */ public String createContainer(String name) { HostConfig hostConfig = HostConfig.newHostConfig() - .withAutoRemove(true) - .withInit(true) - .withCapDrop(Capability.ALL) - .withNetworkMode("none") - .withPidsLimit(2000L) - .withReadonlyRootfs(true) - .withMemory((long) config.dockerMaxRamMegaBytes() * 1024 * 1024) - .withCpuCount((long) Math.ceil(config.dockerCPUsUsage())) - .withCpusetCpus(config.dockerCPUSetCPUs()); + .withAutoRemove(true) + .withInit(true) + .withCapDrop(Capability.ALL) + .withNetworkMode("none") + .withPidsLimit(2000L) + .withReadonlyRootfs(true) + .withMemory((long) config.dockerMaxRamMegaBytes() * 1024 * 1024) + .withCpuCount((long) Math.ceil(config.dockerCPUsUsage())) + .withCpusetCpus(config.dockerCPUSetCPUs()); return client.createContainerCmd(jshellWrapperBaseImageName + Config.JSHELL_WRAPPER_IMAGE_NAME_TAG) .withHostConfig(hostConfig) @@ -140,14 +141,15 @@ public String createContainer(String name) { * @param startupScriptId Script to initialize the container with. * @return The ContainerState of the newly created container. */ - public ContainerState initializeContainer(String name, StartupScriptId startupScriptId) throws IOException { + public ContainerState initializeContainer(String name, StartupScriptId startupScriptId) + throws IOException { if (cachedContainers.isEmpty() || !cachedContainers.containsKey(startupScriptId)) { String containerId = createContainer(name); return setupContainerWithScript(containerId, true, startupScriptId); } String containerId = cachedContainers.get(startupScriptId); executor.submit(() -> initializeCachedContainer(startupScriptId)); - // Rename container with new name. + client.renameContainerCmd(containerId).withName(name).exec(); return setupContainerWithScript(containerId, false, startupScriptId); } @@ -163,7 +165,8 @@ private void initializeCachedContainer(StartupScriptId startupScriptId) { startContainer(id); try (PipedInputStream containerInput = new PipedInputStream(); - BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(new PipedOutputStream(containerInput)))) { + BufferedWriter writer = new BufferedWriter( + new OutputStreamWriter(new PipedOutputStream(containerInput)))) { attachToContainer(id, containerInput); writer.write(Utils.sanitizeStartupScript(startupScriptsService.get(startupScriptId))); @@ -185,12 +188,14 @@ private void initializeCachedContainer(StartupScriptId startupScriptId) { * @return ContainerState of the spawned container. * @throws IOException if an I/O error occurs */ - private ContainerState setupContainerWithScript(String containerId, boolean isCached, StartupScriptId startupScriptId) throws IOException { + private ContainerState setupContainerWithScript(String containerId, boolean isCached, + StartupScriptId startupScriptId) throws IOException { if (!isCached) { startContainer(containerId); } PipedInputStream containerInput = new PipedInputStream(); - BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(new PipedOutputStream(containerInput))); + BufferedWriter writer = + new BufferedWriter(new OutputStreamWriter(new PipedOutputStream(containerInput))); InputStream containerOutput = attachToContainer(containerId, containerInput); BufferedReader reader = new BufferedReader(new InputStreamReader(containerOutput)); @@ -206,6 +211,7 @@ private ContainerState setupContainerWithScript(String containerId, boolean isCa /** * Creates a new container + * * @param containerId the ID of the container to start */ public void startContainer(String containerId) { @@ -219,35 +225,38 @@ public void startContainer(String containerId) { * Logs any output from stderr and returns an InputStream to read stdout. * * @param containerId the ID of the running container to attach to - * @param containerInput the input stream (containerInput) to send to the container + * @param containerInput the input stream (containerInput) to send to the container * @return InputStream to read the container's stdout * @throws IOException if an I/O error occurs */ - public InputStream attachToContainer(String containerId, InputStream containerInput) throws IOException { + public InputStream attachToContainer(String containerId, InputStream containerInput) + throws IOException { PipedInputStream pipeIn = new PipedInputStream(); PipedOutputStream pipeOut = new PipedOutputStream(pipeIn); client.attachContainerCmd(containerId) - .withLogs(true) - .withFollowStream(true) - .withStdOut(true) - .withStdErr(true) - .withStdIn(containerInput) - .exec(new ResultCallback.Adapter<>() { - @Override - public void onNext(Frame object) { - try { - String payloadString = new String(object.getPayload(), StandardCharsets.UTF_8); - if (object.getStreamType() == StreamType.STDOUT) { - pipeOut.write(object.getPayload()); // Write stdout data to pipeOut - } else { - LOGGER.warn("Received STDERR from container {}: {}", containerId, payloadString); - } - } catch (IOException e) { - throw new UncheckedIOException(e); + .withLogs(true) + .withFollowStream(true) + .withStdOut(true) + .withStdErr(true) + .withStdIn(containerInput) + .exec(new ResultCallback.Adapter<>() { + @Override + public void onNext(Frame object) { + try { + String payloadString = + new String(object.getPayload(), StandardCharsets.UTF_8); + if (object.getStreamType() == StreamType.STDOUT) { + pipeOut.write(object.getPayload()); // Write stdout data to pipeOut + } else { + LOGGER.warn("Received STDERR from container {}: {}", containerId, + payloadString); } + } catch (IOException e) { + throw new UncheckedIOException(e); } - }); + } + }); return pipeIn; } diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellService.java index 339b9e8..05108d2 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellService.java @@ -2,14 +2,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.lang.Nullable; import org.togetherjava.jshellapi.Config; import org.togetherjava.jshellapi.dto.*; import org.togetherjava.jshellapi.exceptions.DockerException; import java.io.*; -import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.List; @@ -31,12 +29,8 @@ public class JShellService { private final DockerService dockerService; private final int startupScriptSize; - public JShellService( - DockerService dockerService, - JShellSessionService sessionService, - SessionInfo sessionInfo, - Config config - ) throws DockerException { + public JShellService(DockerService dockerService, JShellSessionService sessionService, + SessionInfo sessionInfo, Config config) throws DockerException { this.dockerService = dockerService; this.sessionService = sessionService; this.id = sessionInfo.id(); @@ -52,7 +46,8 @@ public JShellService( } try { - ContainerState containerState = dockerService.initializeContainer(containerName(), sessionInfo.startupScriptId()); + ContainerState containerState = dockerService.initializeContainer(containerName(), + sessionInfo.startupScriptId()); this.writer = containerState.containerInput(); this.reader = containerState.containerOutput(); checkContainerOK(); diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellSessionService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellSessionService.java index c189551..4a73ea2 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellSessionService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/JShellSessionService.java @@ -90,7 +90,6 @@ private synchronized JShellService createSession(SessionInfo sessionInfo) LOGGER.info("Creating session : {}.", sessionInfo); JShellService service = new JShellService(dockerService, this, sessionInfo, config); - jshellSessions.put(sessionInfo.id(), service); return service; } From 94ffa88e320e393032da291445556de5ae827639 Mon Sep 17 00:00:00 2001 From: Emmanuel Stanley Date: Thu, 17 Oct 2024 13:28:26 +0100 Subject: [PATCH 3/6] Fix conflicts and successfully rebase with upstream develop. --- .../jshellapi/service/DockerService.java | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java index a48e656..3b573cc 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java @@ -27,8 +27,6 @@ public class DockerService implements DisposableBean { private static final Logger LOGGER = LoggerFactory.getLogger(DockerService.class); private static final String WORKER_LABEL = "jshell-api-worker"; private static final UUID WORKER_UNIQUE_ID = UUID.randomUUID(); - private static final String IMAGE_NAME = "togetherjava.org:5001/togetherjava/jshellwrapper"; - private static final String IMAGE_TAG = "master"; private final DockerClient client; private final Config config; @@ -39,7 +37,8 @@ public class DockerService implements DisposableBean { private final String jshellWrapperBaseImageName; - public DockerService(Config config, StartupScriptsService startupScriptsService) throws InterruptedException, IOException { + public DockerService(Config config, StartupScriptsService startupScriptsService) + throws InterruptedException, IOException { this.startupScriptsService = startupScriptsService; DefaultDockerClientConfig clientConfig = DefaultDockerClientConfig.createDefaultConfigBuilder().build(); @@ -83,11 +82,11 @@ private void cleanupLeftovers(UUID currentId) { */ private boolean isImagePresentLocally() { return client.listImagesCmd() - .withFilter("reference", List.of(jshellWrapperBaseImageName)) - .exec() - .stream() - .flatMap(it -> Arrays.stream(it.getRepoTags())) - .anyMatch(it -> it.endsWith(Config.JSHELL_WRAPPER_IMAGE_NAME_TAG)); + .withFilter("reference", List.of(jshellWrapperBaseImageName)) + .exec() + .stream() + .flatMap(it -> Arrays.stream(it.getRepoTags())) + .anyMatch(it -> it.endsWith(Config.JSHELL_WRAPPER_IMAGE_NAME_TAG)); } /** @@ -96,9 +95,9 @@ private boolean isImagePresentLocally() { private void pullImage() throws InterruptedException { if (!isImagePresentLocally()) { client.pullImageCmd(jshellWrapperBaseImageName) - .withTag(IMAGE_TAG) - .exec(new PullImageResultCallback()) - .awaitCompletion(5, TimeUnit.MINUTES); + .withTag("master") + .exec(new PullImageResultCallback()) + .awaitCompletion(5, TimeUnit.MINUTES); } } @@ -120,18 +119,19 @@ public String createContainer(String name) { .withCpuCount((long) Math.ceil(config.dockerCPUsUsage())) .withCpusetCpus(config.dockerCPUSetCPUs()); - return client.createContainerCmd(jshellWrapperBaseImageName + Config.JSHELL_WRAPPER_IMAGE_NAME_TAG) - .withHostConfig(hostConfig) - .withStdinOpen(true) - .withAttachStdin(true) - .withAttachStderr(true) - .withAttachStdout(true) - .withEnv("evalTimeoutSeconds=" + config.evalTimeoutSeconds(), - "sysOutCharLimit=" + config.sysOutCharLimit()) - .withLabels(Map.of(WORKER_LABEL, WORKER_UNIQUE_ID.toString())) - .withName(name) - .exec() - .getId(); + return client + .createContainerCmd(jshellWrapperBaseImageName + Config.JSHELL_WRAPPER_IMAGE_NAME_TAG) + .withHostConfig(hostConfig) + .withStdinOpen(true) + .withAttachStdin(true) + .withAttachStderr(true) + .withAttachStdout(true) + .withEnv("evalTimeoutSeconds=" + config.evalTimeoutSeconds(), + "sysOutCharLimit=" + config.sysOutCharLimit()) + .withLabels(Map.of(WORKER_LABEL, WORKER_UNIQUE_ID.toString())) + .withName(name) + .exec() + .getId(); } /** @@ -143,7 +143,7 @@ public String createContainer(String name) { */ public ContainerState initializeContainer(String name, StartupScriptId startupScriptId) throws IOException { - if (cachedContainers.isEmpty() || !cachedContainers.containsKey(startupScriptId)) { + if (startupScriptId == null || cachedContainers.isEmpty() || !cachedContainers.containsKey(startupScriptId)) { String containerId = createContainer(name); return setupContainerWithScript(containerId, true, startupScriptId); } From 2758d1ae3029121b3035a728857683a36eb749f1 Mon Sep 17 00:00:00 2001 From: Emmanuel Stanley Date: Thu, 17 Oct 2024 15:01:36 +0100 Subject: [PATCH 4/6] fix: Fix Logic Error in Container Creation Leading to Test Case Failure --- .../jshellapi/service/DockerService.java | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java index 3b573cc..463cad2 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java @@ -38,7 +38,7 @@ public class DockerService implements DisposableBean { private final String jshellWrapperBaseImageName; public DockerService(Config config, StartupScriptsService startupScriptsService) - throws InterruptedException, IOException { + throws InterruptedException { this.startupScriptsService = startupScriptsService; DefaultDockerClientConfig clientConfig = DefaultDockerClientConfig.createDefaultConfigBuilder().build(); @@ -143,15 +143,16 @@ public String createContainer(String name) { */ public ContainerState initializeContainer(String name, StartupScriptId startupScriptId) throws IOException { - if (startupScriptId == null || cachedContainers.isEmpty() || !cachedContainers.containsKey(startupScriptId)) { + if (startupScriptId == null || cachedContainers.isEmpty() + || !cachedContainers.containsKey(startupScriptId)) { String containerId = createContainer(name); - return setupContainerWithScript(containerId, true, startupScriptId); + return setupContainerWithScript(containerId, false, startupScriptId); } String containerId = cachedContainers.get(startupScriptId); executor.submit(() -> initializeCachedContainer(startupScriptId)); client.renameContainerCmd(containerId).withName(name).exec(); - return setupContainerWithScript(containerId, false, startupScriptId); + return setupContainerWithScript(containerId, true, startupScriptId); } /** @@ -167,11 +168,12 @@ private void initializeCachedContainer(StartupScriptId startupScriptId) { try (PipedInputStream containerInput = new PipedInputStream(); BufferedWriter writer = new BufferedWriter( new OutputStreamWriter(new PipedOutputStream(containerInput)))) { - attachToContainer(id, containerInput); + InputStream containerOutput = attachToContainer(id, containerInput, true); writer.write(Utils.sanitizeStartupScript(startupScriptsService.get(startupScriptId))); writer.newLine(); writer.flush(); + containerOutput.close(); cachedContainers.put(startupScriptId, id); } catch (IOException e) { @@ -197,7 +199,7 @@ private ContainerState setupContainerWithScript(String containerId, boolean isCa BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(new PipedOutputStream(containerInput))); - InputStream containerOutput = attachToContainer(containerId, containerInput); + InputStream containerOutput = attachToContainer(containerId, containerInput, false); BufferedReader reader = new BufferedReader(new InputStreamReader(containerOutput)); if (!isCached) { @@ -224,13 +226,14 @@ public void startContainer(String containerId) { * Attaches to a running Docker container's input (stdin) and output streams (stdout, stderr). * Logs any output from stderr and returns an InputStream to read stdout. * - * @param containerId the ID of the running container to attach to - * @param containerInput the input stream (containerInput) to send to the container + * @param containerId The ID of the running container to attach to. + * @param containerInput The input stream (containerInput) to send to the container. + * @param isCached Indicator if the container is cached to prevent writing to output stream. * @return InputStream to read the container's stdout * @throws IOException if an I/O error occurs */ - public InputStream attachToContainer(String containerId, InputStream containerInput) - throws IOException { + public InputStream attachToContainer(String containerId, InputStream containerInput, + boolean isCached) throws IOException { PipedInputStream pipeIn = new PipedInputStream(); PipedOutputStream pipeOut = new PipedOutputStream(pipeIn); @@ -247,7 +250,9 @@ public void onNext(Frame object) { String payloadString = new String(object.getPayload(), StandardCharsets.UTF_8); if (object.getStreamType() == StreamType.STDOUT) { - pipeOut.write(object.getPayload()); // Write stdout data to pipeOut + if (!isCached) { + pipeOut.write(object.getPayload()); // Write stdout data to pipeOut + } } else { LOGGER.warn("Received STDERR from container {}: {}", containerId, payloadString); From e8d86efb72d011d45f3e829b74a210fd9159cbac Mon Sep 17 00:00:00 2001 From: Emmanuel Stanley Date: Thu, 17 Oct 2024 22:04:29 +0100 Subject: [PATCH 5/6] Persist input/output streams for containers and fix test case issues - Added functionality to persist input and output streams of newly created or cached Docker containers using a ConcurrentHashMap. - Ensured that container streams are properly managed and retrievable for future requests, improving container reuse. - Fixed test case issues related to stream management and container lifecycle, ensuring tests pass consistently. --- .../jshellapi/dto/ContainerState.java | 9 ++- .../jshellapi/service/DockerService.java | 68 +++++++------------ 2 files changed, 32 insertions(+), 45 deletions(-) diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java index 695baf2..8fd541b 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/ContainerState.java @@ -3,6 +3,13 @@ import java.io.BufferedReader; import java.io.BufferedWriter; -public record ContainerState(boolean isCached, String containerId, BufferedReader containerOutput, +/** + * Data record for the state of a container. + * + * @param containerId The id of the container. + * @param containerOutput The output of the container. + * @param containerInput The input of the container. + */ +public record ContainerState(String containerId, BufferedReader containerOutput, BufferedWriter containerInput) { } diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java index 463cad2..8b4404b 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java @@ -31,7 +31,7 @@ public class DockerService implements DisposableBean { private final DockerClient client; private final Config config; private final ExecutorService executor = Executors.newSingleThreadExecutor(); - private final ConcurrentHashMap cachedContainers = + private final ConcurrentHashMap cachedContainers = new ConcurrentHashMap<>(); private final StartupScriptsService startupScriptsService; @@ -93,12 +93,10 @@ private boolean isImagePresentLocally() { * Pulls the Docker image. */ private void pullImage() throws InterruptedException { - if (!isImagePresentLocally()) { - client.pullImageCmd(jshellWrapperBaseImageName) - .withTag("master") - .exec(new PullImageResultCallback()) - .awaitCompletion(5, TimeUnit.MINUTES); - } + client.pullImageCmd(jshellWrapperBaseImageName) + .withTag("master") + .exec(new PullImageResultCallback()) + .awaitCompletion(5, TimeUnit.MINUTES); } /** @@ -107,7 +105,7 @@ private void pullImage() throws InterruptedException { * @param name The name of the container to create. * @return The ID of the created container. */ - public String createContainer(String name) { + private String createContainer(String name) { HostConfig hostConfig = HostConfig.newHostConfig() .withAutoRemove(true) .withInit(true) @@ -146,13 +144,13 @@ public ContainerState initializeContainer(String name, StartupScriptId startupSc if (startupScriptId == null || cachedContainers.isEmpty() || !cachedContainers.containsKey(startupScriptId)) { String containerId = createContainer(name); - return setupContainerWithScript(containerId, false, startupScriptId); + return setupContainerWithScript(containerId, startupScriptId); } - String containerId = cachedContainers.get(startupScriptId); + ContainerState containerState = cachedContainers.get(startupScriptId); executor.submit(() -> initializeCachedContainer(startupScriptId)); - client.renameContainerCmd(containerId).withName(name).exec(); - return setupContainerWithScript(containerId, true, startupScriptId); + client.renameContainerCmd(containerState.containerId()).withName(name).exec(); + return containerState; } /** @@ -165,17 +163,9 @@ private void initializeCachedContainer(StartupScriptId startupScriptId) { String id = createContainer(containerName); startContainer(id); - try (PipedInputStream containerInput = new PipedInputStream(); - BufferedWriter writer = new BufferedWriter( - new OutputStreamWriter(new PipedOutputStream(containerInput)))) { - InputStream containerOutput = attachToContainer(id, containerInput, true); - - writer.write(Utils.sanitizeStartupScript(startupScriptsService.get(startupScriptId))); - writer.newLine(); - writer.flush(); - containerOutput.close(); - - cachedContainers.put(startupScriptId, id); + try { + ContainerState containerState = setupContainerWithScript(id, startupScriptId); + cachedContainers.put(startupScriptId, containerState); } catch (IOException e) { killContainerByName(containerName); throw new RuntimeException(e); @@ -183,32 +173,26 @@ private void initializeCachedContainer(StartupScriptId startupScriptId) { } /** - * * @param containerId The id of the container - * @param isCached Indicator if the container is cached or new * @param startupScriptId The startup script id of the session * @return ContainerState of the spawned container. * @throws IOException if an I/O error occurs */ - private ContainerState setupContainerWithScript(String containerId, boolean isCached, + private ContainerState setupContainerWithScript(String containerId, StartupScriptId startupScriptId) throws IOException { - if (!isCached) { - startContainer(containerId); - } + startContainer(containerId); PipedInputStream containerInput = new PipedInputStream(); BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(new PipedOutputStream(containerInput))); - InputStream containerOutput = attachToContainer(containerId, containerInput, false); + InputStream containerOutput = attachToContainer(containerId, containerInput); BufferedReader reader = new BufferedReader(new InputStreamReader(containerOutput)); - if (!isCached) { - writer.write(Utils.sanitizeStartupScript(startupScriptsService.get(startupScriptId))); - writer.newLine(); - writer.flush(); - } + writer.write(Utils.sanitizeStartupScript(startupScriptsService.get(startupScriptId))); + writer.newLine(); + writer.flush(); - return new ContainerState(isCached, containerId, reader, writer); + return new ContainerState(containerId, reader, writer); } /** @@ -216,7 +200,7 @@ private ContainerState setupContainerWithScript(String containerId, boolean isCa * * @param containerId the ID of the container to start */ - public void startContainer(String containerId) { + private void startContainer(String containerId) { if (!isContainerRunning(containerId)) { client.startContainerCmd(containerId).exec(); } @@ -228,12 +212,11 @@ public void startContainer(String containerId) { * * @param containerId The ID of the running container to attach to. * @param containerInput The input stream (containerInput) to send to the container. - * @param isCached Indicator if the container is cached to prevent writing to output stream. * @return InputStream to read the container's stdout * @throws IOException if an I/O error occurs */ - public InputStream attachToContainer(String containerId, InputStream containerInput, - boolean isCached) throws IOException { + private InputStream attachToContainer(String containerId, InputStream containerInput) + throws IOException { PipedInputStream pipeIn = new PipedInputStream(); PipedOutputStream pipeOut = new PipedOutputStream(pipeIn); @@ -250,9 +233,7 @@ public void onNext(Frame object) { String payloadString = new String(object.getPayload(), StandardCharsets.UTF_8); if (object.getStreamType() == StreamType.STDOUT) { - if (!isCached) { - pipeOut.write(object.getPayload()); // Write stdout data to pipeOut - } + pipeOut.write(object.getPayload()); // Write stdout data to pipeOut } else { LOGGER.warn("Received STDERR from container {}: {}", containerId, payloadString); @@ -262,7 +243,6 @@ public void onNext(Frame object) { } } }); - return pipeIn; } From e40b62b9346feebff33c248d50d4aeb1dde6cc5a Mon Sep 17 00:00:00 2001 From: Emmanuel Stanley Date: Tue, 22 Oct 2024 20:23:55 +0100 Subject: [PATCH 6/6] chore: address requested changes and enhance logging --- .../jshellapi/service/DockerService.java | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java index 8b4404b..66981bb 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java @@ -11,6 +11,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.DisposableBean; +import org.springframework.lang.Nullable; import org.springframework.stereotype.Service; import org.togetherjava.jshellapi.Config; @@ -58,7 +59,7 @@ public DockerService(Config config, StartupScriptsService startupScriptsService) pullImage(); } cleanupLeftovers(WORKER_UNIQUE_ID); - executor.submit(() -> initializeCachedContainer(StartupScriptId.EMPTY)); + executor.submit(() -> initializeCachedContainer(StartupScriptId.CUSTOM_DEFAULT)); } private void cleanupLeftovers(UUID currentId) { @@ -106,6 +107,7 @@ private void pullImage() throws InterruptedException { * @return The ID of the created container. */ private String createContainer(String name) { + LOGGER.debug("Creating container '{}'", name); HostConfig hostConfig = HostConfig.newHostConfig() .withAutoRemove(true) .withInit(true) @@ -137,10 +139,13 @@ private String createContainer(String name) { * * @param name Name of the container. * @param startupScriptId Script to initialize the container with. + * @throws IOException if an I/O error occurs. * @return The ContainerState of the newly created container. */ - public ContainerState initializeContainer(String name, StartupScriptId startupScriptId) - throws IOException { + public ContainerState initializeContainer(String name, + @Nullable StartupScriptId startupScriptId) throws IOException { + LOGGER.info("Initializing container '{}' with Startup script ID: {}", name, + startupScriptId); if (startupScriptId == null || cachedContainers.isEmpty() || !cachedContainers.containsKey(startupScriptId)) { String containerId = createContainer(name); @@ -159,7 +164,8 @@ public ContainerState initializeContainer(String name, StartupScriptId startupSc * @param startupScriptId Script to initialize the container with. */ private void initializeCachedContainer(StartupScriptId startupScriptId) { - String containerName = cachedContainerName(); + LOGGER.info("Initializing cached container with Startup script ID: {}", startupScriptId); + String containerName = newCachedContainerName(); String id = createContainer(containerName); startContainer(id); @@ -167,12 +173,16 @@ private void initializeCachedContainer(StartupScriptId startupScriptId) { ContainerState containerState = setupContainerWithScript(id, startupScriptId); cachedContainers.put(startupScriptId, containerState); } catch (IOException e) { + LOGGER.error("Could not initialize container {}", id, e); killContainerByName(containerName); throw new RuntimeException(e); } } /** + * Setup container with startup script and also initializes input and output streams for the + * container. + * * @param containerId The id of the container * @param startupScriptId The startup script id of the session * @return ContainerState of the spawned container. @@ -180,6 +190,8 @@ private void initializeCachedContainer(StartupScriptId startupScriptId) { */ private ContainerState setupContainerWithScript(String containerId, StartupScriptId startupScriptId) throws IOException { + LOGGER.info("Setting up container with id {} with Startup script ID: {}", containerId, + startupScriptId); startContainer(containerId); PipedInputStream containerInput = new PipedInputStream(); BufferedWriter writer = @@ -201,9 +213,13 @@ private ContainerState setupContainerWithScript(String containerId, * @param containerId the ID of the container to start */ private void startContainer(String containerId) { - if (!isContainerRunning(containerId)) { - client.startContainerCmd(containerId).exec(); + boolean isRunning = isContainerRunning(containerId); + if (isRunning) { + LOGGER.debug("Container {} is already running.", containerId); + return; } + LOGGER.debug("Container {} is not running. Starting it now.", containerId); + client.startContainerCmd(containerId).exec(); } /** @@ -233,7 +249,7 @@ public void onNext(Frame object) { String payloadString = new String(object.getPayload(), StandardCharsets.UTF_8); if (object.getStreamType() == StreamType.STDOUT) { - pipeOut.write(object.getPayload()); // Write stdout data to pipeOut + pipeOut.write(object.getPayload()); } else { LOGGER.warn("Received STDERR from container {}: {}", containerId, payloadString); @@ -257,7 +273,7 @@ public boolean isContainerRunning(String containerId) { return Boolean.TRUE.equals(containerResponse.getState().getRunning()); } - private String cachedContainerName() { + private String newCachedContainerName() { return "cached_session_" + UUID.randomUUID(); }