Skip to content

Commit

Permalink
Kube: Better Port Abstraction. (#4829)
Browse files Browse the repository at this point in the history
Introduce a better port abstraction whose primary purpose is to confirm that ports are released when the Kube Pod Process is closed.

This prevents issues like #4660

I'm also opening more ports so we can run at least 10 syncs in parallel.
  • Loading branch information
davinchia authored and gl-pix committed Jul 22, 2021
1 parent 09566d9 commit 45fb8da
Show file tree
Hide file tree
Showing 10 changed files with 144 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import io.airbyte.config.helpers.LogClientSingleton;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
Expand Down Expand Up @@ -213,9 +214,11 @@ public String getTemporalHost() {

@Override
public Set<Integer> getTemporalWorkerPorts() {
return Arrays.stream(getEnvOrDefault(TEMPORAL_WORKER_PORTS, "").split(","))
.map(Integer::valueOf)
.collect(Collectors.toSet());
var ports = getEnvOrDefault(TEMPORAL_WORKER_PORTS, "");
if (ports.isEmpty()) {
return new HashSet<>();
}
return Arrays.stream(ports.split(",")).map(Integer::valueOf).collect(Collectors.toSet());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import io.airbyte.scheduler.persistence.JobPersistence;
import io.airbyte.scheduler.persistence.job_tracker.JobTracker;
import io.airbyte.workers.process.DockerProcessFactory;
import io.airbyte.workers.process.KubePortManagerSingleton;
import io.airbyte.workers.process.KubeProcessFactory;
import io.airbyte.workers.process.ProcessFactory;
import io.airbyte.workers.process.WorkerHeartbeatServer;
Expand All @@ -62,10 +63,8 @@
import java.time.Instant;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
Expand All @@ -88,7 +87,7 @@ public class SchedulerApp {
private static final Logger LOGGER = LoggerFactory.getLogger(SchedulerApp.class);

private static final long GRACEFUL_SHUTDOWN_SECONDS = 30;
private static final int SUBMITTER_NUM_THREADS = Integer.parseInt(new EnvConfigs().getSubmitterNumThreads());
private static int SUBMITTER_NUM_THREADS = Integer.parseInt(new EnvConfigs().getSubmitterNumThreads());
private static final Duration SCHEDULING_DELAY = Duration.ofSeconds(5);
private static final Duration CLEANING_DELAY = Duration.ofHours(2);
private static final ThreadFactory THREAD_FACTORY = new ThreadFactoryBuilder().setNameFormat("worker-%d").build();
Expand Down Expand Up @@ -177,11 +176,10 @@ private static ProcessFactory getProcessBuilderFactory(Configs configs) throws I
if (configs.getWorkerEnvironment() == Configs.WorkerEnvironment.KUBERNETES) {
final ApiClient officialClient = Config.defaultClient();
final KubernetesClient fabricClient = new DefaultKubernetesClient();
final BlockingQueue<Integer> workerPorts = new LinkedBlockingDeque<>(configs.getTemporalWorkerPorts());
final String localIp = InetAddress.getLocalHost().getHostAddress();
final String kubeHeartbeatUrl = localIp + ":" + KUBE_HEARTBEAT_PORT;
LOGGER.info("Using Kubernetes namespace: {}", configs.getKubeNamespace());
return new KubeProcessFactory(configs.getKubeNamespace(), officialClient, fabricClient, kubeHeartbeatUrl, workerPorts);
return new KubeProcessFactory(configs.getKubeNamespace(), officialClient, fabricClient, kubeHeartbeatUrl);
} else {
return new DockerProcessFactory(
configs.getWorkspaceRoot(),
Expand Down Expand Up @@ -222,6 +220,13 @@ public static void main(String[] args) throws IOException, InterruptedException
final JobNotifier jobNotifier = new JobNotifier(configs.getWebappUrl(), configRepository);

if (configs.getWorkerEnvironment() == Configs.WorkerEnvironment.KUBERNETES) {
var supportedWorkers = KubePortManagerSingleton.getSupportedWorkers();
if (supportedWorkers < SUBMITTER_NUM_THREADS) {
LOGGER.warn("{} workers configured with only {} ports available. Insufficient ports. Setting workers to {}.", SUBMITTER_NUM_THREADS,
KubePortManagerSingleton.getNumAvailablePorts(), supportedWorkers);
SUBMITTER_NUM_THREADS = supportedWorkers;
}

Map<String, String> mdc = MDC.getCopyOfContextMap();
Executors.newSingleThreadExecutor().submit(
() -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,12 @@ static void gentleCloseWithHeartbeat(final Process process,
final Duration checkHeartbeatDuration,
final Duration forcedShutdownDuration,
final BiConsumer<Process, Duration> forceShutdown) {

while (process.isAlive() && heartbeatMonitor.isBeating()) {
try {
if (new EnvConfigs().getWorkerEnvironment().equals(WorkerEnvironment.KUBERNETES)) {
LOGGER.debug("Gently closing process {} with heartbeat..", process.info().commandLine().get());
}

process.waitFor(checkHeartbeatDuration.toMillis(), TimeUnit.MILLISECONDS);
} catch (InterruptedException e) {
LOGGER.error("Exception while waiting for process to finish", e);
Expand All @@ -125,6 +125,7 @@ static void gentleCloseWithHeartbeat(final Process process,
if (new EnvConfigs().getWorkerEnvironment().equals(WorkerEnvironment.KUBERNETES)) {
LOGGER.debug("Gently closing process {} without heartbeat..", process.info().commandLine().get());
}

process.waitFor(gracefulShutdownDuration.toMillis(), TimeUnit.MILLISECONDS);
} catch (InterruptedException e) {
LOGGER.error("Exception during grace period for process to finish. This can happen when cancelling jobs.");
Expand All @@ -136,6 +137,7 @@ static void gentleCloseWithHeartbeat(final Process process,
if (new EnvConfigs().getWorkerEnvironment().equals(WorkerEnvironment.KUBERNETES)) {
LOGGER.debug("Force shutdown process {}..", process.info().commandLine().get());
}

forceShutdown.accept(process, forcedShutdownDuration);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import org.apache.commons.io.output.NullOutputStream;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -136,7 +135,6 @@ public class KubePodProcess extends Process {
private Integer returnCode = null;
private Long lastStatusCheck = null;

private final Consumer<Integer> portReleaser;
private final ServerSocket stdoutServerSocket;
private final int stdoutLocalPort;
private final ServerSocket stderrServerSocket;
Expand Down Expand Up @@ -286,7 +284,6 @@ private static void waitForInitPodToRun(KubernetesClient client, Pod podDefiniti

public KubePodProcess(ApiClient officialClient,
KubernetesClient fabricClient,
Consumer<Integer> portReleaser,
String podName,
String namespace,
String image,
Expand All @@ -300,7 +297,6 @@ public KubePodProcess(ApiClient officialClient,
final String... args)
throws IOException, InterruptedException {
this.fabricClient = fabricClient;
this.portReleaser = portReleaser;
this.stdoutLocalPort = stdoutLocalPort;
this.stderrLocalPort = stderrLocalPort;

Expand Down Expand Up @@ -528,20 +524,40 @@ public Info info() {

/**
* Close all open resource in the opposite order of resource creation.
*
* Null checks exist because certain local Kube clusters (e.g. Docker for Desktop) back this
* implementation with OS processes and resources, which are automatically reaped by the OS.
*/
private void close() {
Exceptions.swallow(this.stdin::close);
Exceptions.swallow(this.stdout::close);
Exceptions.swallow(this.stderr::close);
if (this.stdin != null) {
Exceptions.swallow(this.stdin::close);
}
if (this.stdout != null) {
Exceptions.swallow(this.stdout::close);
}
if (this.stderr != null) {
Exceptions.swallow(this.stderr::close);
}
Exceptions.swallow(this.stdoutServerSocket::close);
Exceptions.swallow(this.stderrServerSocket::close);
Exceptions.swallow(this.executorService::shutdownNow);
try {
portReleaser.accept(stdoutLocalPort);
portReleaser.accept(stderrLocalPort);
} catch (Exception e) {
LOGGER.error("Error releasing ports ", e);

var stdoutPortReleased = KubePortManagerSingleton.offer(stdoutLocalPort);
if (!stdoutPortReleased) {
LOGGER.warn(
"Error while releasing port {} from pod {}. This can cause the scheduler to run out of ports. Ignore this error if running Kubernetes on docker for desktop.",
stdoutLocalPort,
podDefinition.getMetadata().getName());
}

var stderrPortReleased = KubePortManagerSingleton.offer(stderrLocalPort);
if (!stderrPortReleased) {
LOGGER.warn(
"Error while releasing port {} from pod {}. This can cause the scheduler to run out of ports. Ignore this error if running Kubernetes on docker for desktop",
stderrLocalPort,
podDefinition.getMetadata().getName());
}

LOGGER.debug("Closed {}", podDefinition.getMetadata().getName());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* MIT License
*
* Copyright (c) 2020 Airbyte
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/

package io.airbyte.workers.process;

import com.google.common.annotations.VisibleForTesting;
import io.airbyte.config.EnvConfigs;
import java.util.Set;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.TimeUnit;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Convenience wrapper around a thread-safe BlockingQueue. Keeps track of available ports for Kube
* Pod Processes.
*
* Although this data structure can do without the wrapper class, this class allows easier testing
* via the {@link #getNumAvailablePorts()} function.
*
* The singleton pattern clarifies that only one copy of this class is intended to exists per
* scheduler deployment.
*/
public class KubePortManagerSingleton {

private static final Logger LOGGER = LoggerFactory.getLogger(KubePortManagerSingleton.class);
private static final int MAX_PORTS_PER_WORKER = 4; // A sync has two workers. Each worker requires 2 ports.
private static BlockingQueue<Integer> workerPorts = new LinkedBlockingDeque<>(new EnvConfigs().getTemporalWorkerPorts());

public static Integer take() throws InterruptedException {
return workerPorts.poll(10, TimeUnit.MINUTES);
}

public static boolean offer(Integer port) {
if (!workerPorts.contains(port)) {
workerPorts.add(port);
return true;
}
return false;
}

public static int getNumAvailablePorts() {
return workerPorts.size();
}

public static int getSupportedWorkers() {
return workerPorts.size() / MAX_PORTS_PER_WORKER;
}

@VisibleForTesting
protected static void setWorkerPorts(Set<Integer> ports) {
workerPorts = new LinkedBlockingDeque<>(ports);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
import io.kubernetes.client.openapi.ApiClient;
import java.nio.file.Path;
import java.util.Map;
import java.util.concurrent.BlockingQueue;
import java.util.function.Consumer;
import org.apache.commons.lang3.RandomStringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -45,7 +43,6 @@ public class KubeProcessFactory implements ProcessFactory {
private final ApiClient officialClient;
private final KubernetesClient fabricClient;
private final String kubeHeartbeatUrl;
private final BlockingQueue<Integer> workerPorts;

/**
* @param namespace kubernetes namespace where spawned pods will live
Expand All @@ -58,13 +55,11 @@ public class KubeProcessFactory implements ProcessFactory {
public KubeProcessFactory(String namespace,
ApiClient officialClient,
KubernetesClient fabricClient,
String kubeHeartbeatUrl,
BlockingQueue<Integer> workerPorts) {
String kubeHeartbeatUrl) {
this.namespace = namespace;
this.officialClient = officialClient;
this.fabricClient = fabricClient;
this.kubeHeartbeatUrl = kubeHeartbeatUrl;
this.workerPorts = workerPorts;
}

@Override
Expand All @@ -80,27 +75,18 @@ public Process create(String jobId,
throws WorkerException {
try {
// used to differentiate source and destination processes with the same id and attempt

final String podName = createPodName(imageName, jobId, attempt);

final int stdoutLocalPort = workerPorts.take();
final int stdoutLocalPort = KubePortManagerSingleton.take();
LOGGER.info("{} stdoutLocalPort = {}", podName, stdoutLocalPort);

final int stderrLocalPort = workerPorts.take();
final int stderrLocalPort = KubePortManagerSingleton.take();
LOGGER.info("{} stderrLocalPort = {}", podName, stderrLocalPort);

final Consumer<Integer> portReleaser = port -> {
if (!workerPorts.contains(port)) {
workerPorts.add(port);
LOGGER.info("{} releasing: {}", podName, port);
} else {
LOGGER.info("{} skipping releasing: {}", podName, port);
}
};

return new KubePodProcess(
officialClient,
fabricClient,
portReleaser,
podName,
namespace,
imageName,
Expand Down
Loading

0 comments on commit 45fb8da

Please sign in to comment.