diff --git a/lib/compilation-queue.ts b/lib/compilation-queue.ts index aef592493..edac64863 100644 --- a/lib/compilation-queue.ts +++ b/lib/compilation-queue.ts @@ -41,7 +41,7 @@ const queueDequeued = new PromClient.Counter({ }); const queueCompleted = new PromClient.Counter({ name: 'ce_compilation_queue_completed_total', - help: 'Total number of jobs completed', + help: 'Total number of jobs whose promise settled (fulfilled or rejected); a wedged job never counts', }); const queueStale = new PromClient.Counter({ name: 'ce_compilation_queue_stale_total', @@ -102,10 +102,17 @@ export class CompilationQueue { } try { this._running.add(jobAsyncId); - return job(); + const result = job(); + // Count completion when the job settles (even by rejection), not when it + // merely returns its promise. Deliberately not awaited here: a job that + // never settles must not hold _running (and so status().busy) forever. + Promise.resolve(result).then( + () => queueCompleted.inc(), + () => queueCompleted.inc(), + ); + return result; } finally { this._running.delete(jobAsyncId); - queueCompleted.inc(); } }, {priority: options?.highPriority ? 100 : 0}, diff --git a/test/compilation-queue-tests.ts b/test/compilation-queue-tests.ts index 2598e4efd..a31d7049c 100644 --- a/test/compilation-queue-tests.ts +++ b/test/compilation-queue-tests.ts @@ -23,10 +23,17 @@ // POSSIBILITY OF SUCH DAMAGE. import {TimeoutError} from 'p-queue'; +import PromClient from 'prom-client'; import {describe, expect, it} from 'vitest'; import {CompilationQueue} from '../lib/compilation-queue.js'; +async function completedCount(): Promise { + const counter = PromClient.register.getSingleMetric('ce_compilation_queue_completed_total'); + const metric = await (counter as PromClient.Counter).get(); + return metric.values[0].value; +} + describe('CompilationQueue', () => { it('runs an enqueued job and returns its result', async () => { const queue = new CompilationQueue(1, 1000, 1000); @@ -55,4 +62,26 @@ describe('CompilationQueue', () => { // Generous test budget: the 100ms queue timeout can fire very late when vitest workers // saturate the machine (e.g. during pre-commit runs). }, 15_000); + + it('counts a job as completed when it settles, not when it starts', async () => { + const queue = new CompilationQueue(1, 1000, 1000); + const before = await completedCount(); + let release: () => void = () => {}; + const gate = new Promise(resolve => { + release = resolve; + }); + let started: () => void = () => {}; + const startedGate = new Promise(resolve => { + started = resolve; + }); + const jobPromise = queue.enqueue(() => { + started(); + return gate; + }); + await startedGate; + expect(await completedCount()).toEqual(before); + release(); + await jobPromise; + expect(await completedCount()).toEqual(before + 1); + }); });