mirror of
https://github.com/compiler-explorer/compiler-explorer.git
synced 2026-07-16 18:37:07 -04:00
Count queue jobs as completed when they settle, not when dequeued (#8821)
The last loose end from the incident issue — `ce_compilation_queue_completed_total` incremented in a `finally` that ran as soon as the job *returned its promise*, so it counted dequeues, in lockstep with `ce_compilation_queue_dequeued_total`, and `status().running` was almost always 0. Completion is now counted when the job settles (fulfilled or rejected), via a settlement callback rather than by awaiting in the wrapper. The non-awaiting detail matters: my first attempt awaited `job()` so the `finally` ran at settlement — and the existing "times out a job that never settles" test immediately caught that this keeps `_running` populated forever for a wedged job, reintroducing the exact `busy`-forever wedge #8813 fixed. (A nice demonstration of that test paying for itself.) With the callback approach, a never-settling job correctly never counts as completed, so `dequeued − completed` now exposes wedged/in-flight jobs — which would have made the original incident visible directly in Grafana. New test pins the semantics: counter unchanged while a job is running, +1 once it settles. Per discussion: no temp-dir sweeps of any kind (instances are replaced, never restarted; and multiple CE processes may share a machine), so the orphaned-dirs observation in #8811 is closed as won't-fix. Closes #8811. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: mattgodbolt-molty <mattgodbolt-molty@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
parent
51317dc13f
commit
19ae5abcf3
@@ -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},
|
||||
|
||||
@@ -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<number> {
|
||||
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<void>(resolve => {
|
||||
release = resolve;
|
||||
});
|
||||
let started: () => void = () => {};
|
||||
const startedGate = new Promise<void>(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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user