From 4c135c251f59232724c39cd70657c902b422acb2 Mon Sep 17 00:00:00 2001 From: Matt Moyles Date: Thu, 15 Oct 2026 12:55:35 -0400 Subject: [PATCH] fix(install): wait for a dependency that is already installing instead of installing it twice Starting Qdrant and Ollama together from the setup wizard ran two installs of Qdrant: its own, and the one Ollama's dependency check started because the row was not yet marked installed. The second hit Docker's 409 on the container name, and its failure cleanup force-removed the container the first install had created. Depending on timing, both installs then failed, or Qdrant was left marked installed with no container. _createContainer now shares one in-flight run per service name, held in a static map because each request gets its own DockerService. A dependency that is already installing is awaited, and the activity feed says so. Fixes #1439 --- admin/app/services/docker_service.ts | 33 ++++++++++-- admin/app/utils/in_flight.ts | 29 ++++++++++ admin/tests/unit/in_flight.spec.ts | 79 ++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 5 deletions(-) create mode 100644 admin/app/utils/in_flight.ts create mode 100644 admin/tests/unit/in_flight.spec.ts diff --git a/admin/app/services/docker_service.ts b/admin/app/services/docker_service.ts index ca8fc0d2..ef238da2 100644 --- a/admin/app/services/docker_service.ts +++ b/admin/app/services/docker_service.ts @@ -5,6 +5,7 @@ import { inject } from '@adonisjs/core' import transmit from '@adonisjs/transmit/services/main' import { doResumableDownloadWithRetry } from '../utils/downloads.js' import { pickAmdHsaOverride } from '../utils/amd_hsa_override.js' +import { joinInFlight } from '../utils/in_flight.js' import { join } from 'path' import os from 'node:os' import env from '#start/env' @@ -35,6 +36,9 @@ const AMD_GFX_MARKER_PATH = '/app/storage/.nomad-amd-gfx' export class DockerService { public docker: Docker private activeInstallations: Set = new Set() + // Static, not per-instance: controllers get a fresh DockerService per request, and + // the installs that race (#1439) arrive on separate requests. + private static installsInFlight = new Map>() public static NOMAD_NETWORK = 'project-nomad_default' public static ADMIN_CONTAINER_NAME = 'nomad_admin' // The admin's own storage mount destination inside its container (compose maps @@ -627,6 +631,17 @@ export class DockerService { async _createContainer( service: Service & { dependencies?: Service[] }, containerConfig: any + ): Promise { + // A service that is already installing (its own install, or another service's + // dependency install) is joined rather than started again (#1439). + return joinInFlight(DockerService.installsInFlight, service.service_name, () => + this._installContainer(service, containerConfig) + ) + } + + private async _installContainer( + service: Service & { dependencies?: Service[] }, + containerConfig: any ): Promise { try { this._broadcast(service.service_name, 'initializing', '') @@ -652,11 +667,19 @@ export class DockerService { ) for (const dependency of dependencies) { if (!dependency.installed) { - this._broadcast( - service.service_name, - 'dependency-not-installed', - `Dependency service ${dependency.service_name} is not installed. Installing it first...` - ) + if (DockerService.installsInFlight.has(dependency.service_name)) { + this._broadcast( + service.service_name, + 'dependency-installing', + `Dependency service ${dependency.service_name} is already being installed. Waiting for it to finish...` + ) + } else { + this._broadcast( + service.service_name, + 'dependency-not-installed', + `Dependency service ${dependency.service_name} is not installed. Installing it first...` + ) + } await this._createContainer( dependency, this._parseContainerConfig(dependency.container_config) diff --git a/admin/app/utils/in_flight.ts b/admin/app/utils/in_flight.ts new file mode 100644 index 00000000..058c65cb --- /dev/null +++ b/admin/app/utils/in_flight.ts @@ -0,0 +1,29 @@ +/** + * Shares one run of an async task between every caller that asks for the same + * key while it is running. + * + * Used by `DockerService._createContainer` so that two installs of the same + * service can't run side by side (#1439). Installing Ollama and Qdrant together + * from the setup wizard starts Qdrant's own install, then Ollama's dependency + * check, which sees Qdrant not yet installed and starts a second one. The second + * hits Docker's 409 on the container name, and its failure cleanup removes the + * container the first install created. + * + * The run is registered synchronously, before `start` yields, so a caller that + * arrives on the next tick already finds it. It is removed once it settles, + * whether it resolved or rejected, so a failed install can be retried. + */ +export function joinInFlight( + inFlight: Map>, + key: string, + start: () => Promise +): Promise { + const running = inFlight.get(key) + if (running) return running + + const run = start().finally(() => { + if (inFlight.get(key) === run) inFlight.delete(key) + }) + inFlight.set(key, run) + return run +} diff --git a/admin/tests/unit/in_flight.spec.ts b/admin/tests/unit/in_flight.spec.ts new file mode 100644 index 00000000..d7cf0415 --- /dev/null +++ b/admin/tests/unit/in_flight.spec.ts @@ -0,0 +1,79 @@ +import * as assert from 'node:assert/strict' +import { test } from 'node:test' + +import { joinInFlight } from '../../app/utils/in_flight.js' + +function deferred() { + let resolve!: () => void + let reject!: (err: Error) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} + +test('a second install of a service that is still installing joins the first (#1439)', async () => { + const inFlight = new Map>() + const qdrant = deferred() + let starts = 0 + const install = () => { + starts++ + return qdrant.promise + } + + // Qdrant's own install, then Ollama's dependency check for Qdrant, on the next tick. + const own = joinInFlight(inFlight, 'nomad_qdrant', install) + await Promise.resolve() + const fromDependency = joinInFlight(inFlight, 'nomad_qdrant', install) + + assert.equal(starts, 1) + assert.equal(fromDependency, own) + + qdrant.resolve() + await fromDependency + assert.equal(inFlight.has('nomad_qdrant'), false) +}) + +test('a joined install that fails rejects every caller once, and can be retried', async () => { + const inFlight = new Map>() + const first = deferred() + let starts = 0 + + const a = joinInFlight(inFlight, 'nomad_qdrant', () => { + starts++ + return first.promise + }) + const b = joinInFlight(inFlight, 'nomad_qdrant', () => { + starts++ + return Promise.resolve() + }) + first.reject(new Error('pull failed')) + + await assert.rejects(a, /pull failed/) + await assert.rejects(b, /pull failed/) + assert.equal(starts, 1) + assert.equal(inFlight.has('nomad_qdrant'), false) + + await joinInFlight(inFlight, 'nomad_qdrant', () => { + starts++ + return Promise.resolve() + }) + assert.equal(starts, 2) +}) + +test('installs of different services run independently', async () => { + const inFlight = new Map>() + const qdrant = deferred() + let ollamaStarted = false + + const q = joinInFlight(inFlight, 'nomad_qdrant', () => qdrant.promise) + await joinInFlight(inFlight, 'nomad_ollama', async () => { + ollamaStarted = true + }) + + assert.equal(ollamaStarted, true) + assert.equal(inFlight.has('nomad_qdrant'), true) + qdrant.resolve() + await q +})