Let cancel terminate auxiliary build phases instead of blocking the slot
Cancelling a build only killed the running build process; the synchronous preparation phases — most notably a multi-minute Docker image build, but also the Gradle-volume preparation — ran to completion and kept the concurrency slot occupied, so the next queued build stayed PENDING for a long time. Build runners now report every auxiliary process through an onAuxProcess sink (GitCommandRunner gained an onProcess hook), and the executor registers them like the build process, so cancellation terminates whatever is currently running. Measured on vm4006: cancel to next-build-running is ~3s in the normal case; the unit test covers the aux-phase case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
76321a6f5c
commit
ed9562d1a7
@@ -218,6 +218,14 @@ class BuildExecutor(
|
||||
environment = mapOf("branch" to build.runningBuild.branch),
|
||||
repoDir = build.workingDir,
|
||||
branchConfig = branchConfig,
|
||||
onAuxProcess = { aux ->
|
||||
// preparation phases (e.g. a Docker image build) must die on cancellation
|
||||
// too, otherwise a cancelled build blocks its slot until they finish
|
||||
build.process = aux
|
||||
if (build.cancelled.get()) {
|
||||
destroyProcessTree(aux)
|
||||
}
|
||||
},
|
||||
)
|
||||
build.process = process
|
||||
if (build.cancelled.get()) {
|
||||
|
||||
@@ -10,6 +10,9 @@ import java.nio.file.Path
|
||||
* which owns log streaming and process-tree termination.
|
||||
* [repoDir] and [branchConfig] let implementations derive per-repository resources
|
||||
* and per-branch settings (step 11: Docker runtime).
|
||||
* Implementations report every auxiliary process they run before the build command
|
||||
* (e.g. a Docker image build) via [onAuxProcess], so a cancellation can terminate
|
||||
* long preparation phases instead of blocking the build slot until they finish.
|
||||
*/
|
||||
interface BuildRunner {
|
||||
fun start(
|
||||
@@ -18,6 +21,7 @@ interface BuildRunner {
|
||||
environment: Map<String, String>,
|
||||
repoDir: Path = workingDir,
|
||||
branchConfig: BranchConfig = BranchConfig(),
|
||||
onAuxProcess: (Process) -> Unit = {},
|
||||
): Process
|
||||
}
|
||||
|
||||
@@ -29,6 +33,7 @@ class ProcessBuildRunner : BuildRunner {
|
||||
environment: Map<String, String>,
|
||||
repoDir: Path,
|
||||
branchConfig: BranchConfig,
|
||||
onAuxProcess: (Process) -> Unit,
|
||||
): Process {
|
||||
val processBuilder = ProcessBuilder("bash", "-c", command)
|
||||
processBuilder.directory(workingDir.toFile())
|
||||
@@ -53,8 +58,9 @@ class DispatchingBuildRunner(
|
||||
environment: Map<String, String>,
|
||||
repoDir: Path,
|
||||
branchConfig: BranchConfig,
|
||||
onAuxProcess: (Process) -> Unit,
|
||||
): Process {
|
||||
val runner = if (branchConfig.docker.enabled) dockerBuildRunner else processBuildRunner
|
||||
return runner.start(command, workingDir, environment, repoDir, branchConfig)
|
||||
return runner.start(command, workingDir, environment, repoDir, branchConfig, onAuxProcess)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -48,12 +48,13 @@ class DockerBuildRunner(
|
||||
environment: Map<String, String>,
|
||||
repoDir: Path,
|
||||
branchConfig: BranchConfig,
|
||||
onAuxProcess: (Process) -> Unit,
|
||||
): Process {
|
||||
val docker = branchConfig.docker
|
||||
require(docker.image.isNotBlank()) { "branches.<name>.docker.image must be set when docker.enabled is true" }
|
||||
val repoKey = ArtifactKeys.repoKey(repoDir)
|
||||
cleanupStaleContainersOnce(repoKey, repoDir)
|
||||
ensureImage(docker, repoDir)
|
||||
ensureImage(docker, repoDir, onAuxProcess)
|
||||
val uid = id("-u", repoDir)
|
||||
val gid = id("-g", repoDir)
|
||||
val socket = socketLocator.locate(uid)
|
||||
@@ -64,7 +65,7 @@ class DockerBuildRunner(
|
||||
val ownershipUid = if (rootless) "0" else uid
|
||||
val ownershipGid = if (rootless) "0" else gid
|
||||
val volume = gradleVolumeName(repoKey)
|
||||
prepareGradleVolume(volume, docker.image, ownershipUid, ownershipGid, repoDir)
|
||||
prepareGradleVolume(volume, docker.image, ownershipUid, ownershipGid, repoDir, onAuxProcess)
|
||||
val containerName = containerName(repoKey, environment["branch"])
|
||||
removeContainer(containerName, repoDir)
|
||||
val runCommand =
|
||||
@@ -92,6 +93,7 @@ class DockerBuildRunner(
|
||||
private fun ensureImage(
|
||||
docker: DockerConfig,
|
||||
repoDir: Path,
|
||||
onAuxProcess: (Process) -> Unit,
|
||||
) {
|
||||
if (docker.dockerfile.isBlank()) {
|
||||
return
|
||||
@@ -126,6 +128,7 @@ class DockerBuildRunner(
|
||||
docker.context,
|
||||
),
|
||||
repoDir,
|
||||
onProcess = onAuxProcess,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -140,6 +143,7 @@ class DockerBuildRunner(
|
||||
uid: String,
|
||||
gid: String,
|
||||
repoDir: Path,
|
||||
onAuxProcess: (Process) -> Unit,
|
||||
) {
|
||||
val key = "$volume@$image"
|
||||
if (key in preparedVolumes) {
|
||||
@@ -164,6 +168,7 @@ class DockerBuildRunner(
|
||||
gid,
|
||||
),
|
||||
repoDir,
|
||||
onProcess = onAuxProcess,
|
||||
)
|
||||
preparedVolumes += key
|
||||
}
|
||||
|
||||
@@ -23,27 +23,39 @@ class GitCommandException(
|
||||
|
||||
@Component
|
||||
class GitCommandRunner {
|
||||
/** [onProcess] receives the started process, so callers can terminate it early (build cancellation). */
|
||||
fun run(
|
||||
command: List<String>,
|
||||
workingDir: Path,
|
||||
environment: Map<String, String> = emptyMap(),
|
||||
onProcess: (Process) -> Unit = {},
|
||||
): GitCommandResult {
|
||||
val builder = ProcessBuilder(command).directory(workingDir.toFile())
|
||||
builder.environment().putAll(environment)
|
||||
val process = builder.start()
|
||||
// stderr is drained concurrently so neither pipe buffer can block the process
|
||||
val stderr = CompletableFuture.supplyAsync { process.errorStream.bufferedReader().readText() }
|
||||
val stdout = process.inputStream.bufferedReader().readText()
|
||||
onProcess(process)
|
||||
// stderr is drained concurrently so neither pipe buffer can block the process;
|
||||
// reads tolerate closed streams, e.g. when onProcess terminated the process early
|
||||
val stderr = CompletableFuture.supplyAsync { process.errorStream.readSafely() }
|
||||
val stdout = process.inputStream.readSafely()
|
||||
val exitCode = process.waitFor()
|
||||
return GitCommandResult(exitCode, stdout, stderr.get())
|
||||
}
|
||||
|
||||
private fun java.io.InputStream.readSafely(): String =
|
||||
try {
|
||||
bufferedReader().readText()
|
||||
} catch (_: java.io.IOException) {
|
||||
""
|
||||
}
|
||||
|
||||
fun runOrThrow(
|
||||
command: List<String>,
|
||||
workingDir: Path,
|
||||
environment: Map<String, String> = emptyMap(),
|
||||
onProcess: (Process) -> Unit = {},
|
||||
): GitCommandResult {
|
||||
val result = run(command, workingDir, environment)
|
||||
val result = run(command, workingDir, environment, onProcess)
|
||||
if (!result.isSuccess) {
|
||||
throw GitCommandException(command, result)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user