Skip to content

Commit 075327e

Browse files
AztecBotPhilWindle
authored andcommitted
feat(prover): revive cancelled proving jobs from a persisted aborted state (#24578)
v5 version of #24558 (same change, based on `merge-train/spartan-v5`). ## Problem When the prover broker cancelled a proving job it settled the job as a cached `{ status: 'rejected', reason: 'Aborted' }` result — in memory and in the database. Because a job's id is a deterministic hash of its inputs, any later enqueue of the same job returned that cached abort (`Cached proving job … Not enqueuing again`) and the facade failed the job with `Aborted`. So once a proof was aborted it stayed aborted, and a retry — a fresh epoch attempt after a reorg/failure, or a re-orchestration after a restart — got the cached abort instead of re-proving. A planned mid-epoch redeploy hit this and turned into a testnet epoch prune. ## Fix Cancellation is now its own first-class, **revivable** state: - A new `aborted` proving-job status (`ProvingJobStatus` / `ProvingJobSettledResult`). `cancelProvingJob` records it, notifies the current waiter, and **persists** it (`ProvingBrokerDatabase.setProvingJobAborted`). - `enqueueProvingJob` **revives** an aborted job: when the producer re-requests it, the broker clears the aborted state — in memory and, via the new `deleteProvingJobResult`, in the database — and re-enqueues it as if new. So a restart mid-revival keeps the job pending rather than resurrecting the abort. Genuine agent errors and timeouts still settle as terminal `rejected` results. Net effect: cancelling frees the job cleanly and records why, but re-requesting the same proof always brings it back to life — in the same process or after a restart — so an abort can never permanently block an epoch from proving. The stacked PR (prover-node clean-shutdown, v5) stops a clean prover-node shutdown from cancelling its in-flight jobs in the first place. ## Tests `proving_broker.test.ts` (both DB variants): cancel leaves a job `aborted`; re-request revives it and it completes; the aborted state persists across a restart; and a revived job survives a restart mid-revival as pending (not aborted). Note: not run locally in this session — the prover-client suite needs a full noir/wasm bootstrap that wasn't available here — so relied on CI. --- *Created by [claudebox](https://claudebox.work/v2/sessions/6fa5e242ed9ceae7) · group: `slackbot`* --------- Co-authored-by: Phil Windle <philip.windle@gmail.com> (cherry picked from commit a4e88ca)
1 parent a176d96 commit 075327e

8 files changed

Lines changed: 290 additions & 25 deletions

File tree

yarn-project/prover-client/src/proving_broker/broker_prover_facade.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,8 @@ export class BrokerCircuitProverFacade implements ServerCircuitProver {
288288
}
289289
} else if (result.status === 'rejected') {
290290
return { success: false, reason: result.reason };
291+
} else if (result.status === 'aborted') {
292+
return { success: false, reason: 'Aborted' };
291293
} else {
292294
throw new Error(`Unexpected proving job status ${result.status}`);
293295
}

yarn-project/prover-client/src/proving_broker/proving_broker.test.ts

Lines changed: 131 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ describe.each([
201201
await assertJobStatus(id, 'in-queue');
202202

203203
await broker.cancelProvingJob(id);
204-
await assertJobStatus(id, 'rejected');
204+
await assertJobStatus(id, 'aborted');
205205
});
206206

207207
it('cancels jobs in-progress', async () => {
@@ -216,7 +216,135 @@ describe.each([
216216
await broker.getProvingJob();
217217
await assertJobStatus(id, 'in-progress');
218218
await broker.cancelProvingJob(id);
219-
await assertJobStatus(id, 'rejected');
219+
await assertJobStatus(id, 'aborted');
220+
});
221+
222+
it('revives an aborted job when its producer re-requests it', async () => {
223+
const provingJob: ProvingJob = {
224+
id: makeRandomProvingJobId(),
225+
type: ProvingRequestType.PARITY_BASE,
226+
epochNumber: EpochNumber(1),
227+
inputsUri: makeInputsUri(),
228+
};
229+
230+
await broker.enqueueProvingJob(provingJob);
231+
await broker.getProvingJob();
232+
await assertJobStatus(provingJob.id, 'in-progress');
233+
234+
await broker.cancelProvingJob(provingJob.id);
235+
await assertJobStatus(provingJob.id, 'aborted');
236+
237+
// Re-requesting the same job (a retry without a restart) revives it rather than returning the
238+
// cached abort. The job stays cached through the revive, so the start-of-call status is
239+
// 'in-queue', and it can then be completed.
240+
await expect(broker.enqueueProvingJob(provingJob)).resolves.toEqual({ status: 'in-queue' });
241+
await assertJobStatus(provingJob.id, 'in-queue');
242+
const returnedJob = await broker.getProvingJob({ allowList: [ProvingRequestType.PARITY_BASE] });
243+
expect(returnedJob?.job).toEqual(provingJob);
244+
245+
const retryValue = makeOutputsUri();
246+
await broker.reportProvingJobSuccess(provingJob.id, retryValue);
247+
await assertJobStatus(provingJob.id, 'fulfilled');
248+
});
249+
250+
it('persists the aborted state across a restart and revives on re-request', async () => {
251+
const provingJob: ProvingJob = {
252+
id: makeRandomProvingJobId(),
253+
type: ProvingRequestType.PARITY_BASE,
254+
epochNumber: EpochNumber(1),
255+
inputsUri: makeInputsUri(),
256+
};
257+
258+
await broker.enqueueProvingJob(provingJob);
259+
await broker.cancelProvingJob(provingJob.id);
260+
await assertJobStatus(provingJob.id, 'aborted');
261+
262+
// A deploy restarts both the prover node and the broker. The aborted state is persisted, so it
263+
// survives the restart rather than being lost or restored as a permanent rejection.
264+
await broker.stop();
265+
broker = new ProvingBroker(database, {
266+
proverBrokerJobTimeoutMs: jobTimeoutMs,
267+
proverBrokerPollIntervalMs: brokerIntervalMs,
268+
proverBrokerJobMaxRetries: maxRetries,
269+
proverBrokerMaxEpochsToKeepResultsFor: 1,
270+
proverBrokerDebugReplayEnabled: false,
271+
});
272+
await broker.start();
273+
await assertJobStatus(provingJob.id, 'aborted');
274+
275+
// The prover re-requests the job after the restart, which revives it (returning the cached
276+
// 'in-queue' status), and it can be completed.
277+
await expect(broker.enqueueProvingJob(provingJob)).resolves.toEqual({ status: 'in-queue' });
278+
await assertJobStatus(provingJob.id, 'in-queue');
279+
const returnedJob = await broker.getProvingJob({ allowList: [ProvingRequestType.PARITY_BASE] });
280+
expect(returnedJob?.job).toEqual(provingJob);
281+
282+
const value = makeOutputsUri();
283+
await broker.reportProvingJobSuccess(provingJob.id, value);
284+
await assertJobStatus(provingJob.id, 'fulfilled');
285+
});
286+
287+
it('persists the revived (non-aborted) state, so a restart mid-revival stays revived', async () => {
288+
const provingJob: ProvingJob = {
289+
id: makeRandomProvingJobId(),
290+
type: ProvingRequestType.PARITY_BASE,
291+
epochNumber: EpochNumber(1),
292+
inputsUri: makeInputsUri(),
293+
};
294+
295+
await broker.enqueueProvingJob(provingJob);
296+
await broker.cancelProvingJob(provingJob.id);
297+
await assertJobStatus(provingJob.id, 'aborted');
298+
299+
// Revive the job but do not complete it before the broker restarts.
300+
await broker.enqueueProvingJob(provingJob);
301+
await assertJobStatus(provingJob.id, 'in-queue');
302+
303+
await broker.stop();
304+
broker = new ProvingBroker(database, {
305+
proverBrokerJobTimeoutMs: jobTimeoutMs,
306+
proverBrokerPollIntervalMs: brokerIntervalMs,
307+
proverBrokerJobMaxRetries: maxRetries,
308+
proverBrokerMaxEpochsToKeepResultsFor: 1,
309+
proverBrokerDebugReplayEnabled: false,
310+
});
311+
await broker.start();
312+
313+
// Reviving cleared the persisted aborted state, so the job comes back pending rather than
314+
// aborted and keeps being proven without needing another re-request.
315+
await assertJobStatus(provingJob.id, 'in-queue');
316+
});
317+
318+
it('revives once when the producer re-requests an aborted job concurrently', async () => {
319+
const provingJob: ProvingJob = {
320+
id: makeRandomProvingJobId(),
321+
type: ProvingRequestType.PARITY_BASE,
322+
epochNumber: EpochNumber(1),
323+
inputsUri: makeInputsUri(),
324+
};
325+
326+
await broker.enqueueProvingJob(provingJob);
327+
await broker.cancelProvingJob(provingJob.id);
328+
await assertJobStatus(provingJob.id, 'aborted');
329+
330+
// Two overlapping re-requests race through the revive. jobsCache stays populated as the enqueue
331+
// lock throughout, so exactly one of them re-enqueues the job and both observe it as in-queue
332+
// rather than the stale abort. The revived job is then delivered once and completes normally.
333+
const [first, second] = await Promise.all([
334+
broker.enqueueProvingJob(provingJob),
335+
broker.enqueueProvingJob(provingJob),
336+
]);
337+
expect(first).toEqual({ status: 'in-queue' });
338+
expect(second).toEqual({ status: 'in-queue' });
339+
await assertJobStatus(provingJob.id, 'in-queue');
340+
341+
const returnedJob = await broker.getProvingJob({ allowList: [ProvingRequestType.PARITY_BASE] });
342+
expect(returnedJob?.job).toEqual(provingJob);
343+
await assertJobStatus(provingJob.id, 'in-progress');
344+
await expect(broker.getProvingJob({ allowList: [ProvingRequestType.PARITY_BASE] })).resolves.toBeUndefined();
345+
346+
await broker.reportProvingJobSuccess(provingJob.id, makeOutputsUri());
347+
await assertJobStatus(provingJob.id, 'fulfilled');
220348
});
221349

222350
it('returns job result if successful', async () => {
@@ -523,7 +651,7 @@ describe.each([
523651
await broker.getProvingJob();
524652
await assertJobStatus(id, 'in-progress');
525653
await broker.cancelProvingJob(id);
526-
await assertJobStatus(id, 'rejected');
654+
await assertJobStatus(id, 'aborted');
527655

528656
const id2 = makeRandomProvingJobId();
529657
await broker.enqueueProvingJob({

yarn-project/prover-client/src/proving_broker/proving_broker.ts

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -272,15 +272,40 @@ export class ProvingBroker implements ProvingJobProducer, ProvingJobConsumer, Pr
272272

273273
async #enqueueProvingJob(job: ProvingJob): Promise<ProvingJobStatus> {
274274
// We return the job status at the start of this call
275-
const jobStatus = this.#getProvingJobStatus(job.id);
275+
let jobStatus = this.#getProvingJobStatus(job.id);
276276
if (this.jobsCache.has(job.id)) {
277277
const existing = this.jobsCache.get(job.id);
278278
assert.deepStrictEqual(job, existing, 'Duplicate proving job ID');
279-
this.logger.warn(`Cached proving job id=${job.id} epochNumber=${job.epochNumber}. Not enqueuing again`, {
280-
provingJobId: job.id,
281-
});
282-
this.instrumentation.incCachedJobs(job.type);
283-
return jobStatus;
279+
280+
if (this.resultsCache.get(job.id)?.status === 'aborted') {
281+
// The producer is re-requesting a job it previously cancelled: revive it rather than
282+
// returning the cached abort, clearing the aborted state in memory and in the database so the
283+
// revival survives a restart.
284+
//
285+
// Concurrency model: `jobsCache` is the enqueue lock. Every path that puts a job on the queue
286+
// populates `jobsCache` *synchronously, before its first await* (see the "New proving job"
287+
// block below), so a second concurrent enqueue of the same id observes the entry at the top
288+
// of this method and takes a cached, no-op branch instead of enqueuing a duplicate. The revive
289+
// must keep holding that lock: we tear down the settled state and re-set `jobsCache` in a
290+
// single synchronous span (no await in between), and only then await the database. Because a
291+
// concurrent re-request can only interleave at that await — by which point `jobsCache` is
292+
// populated again and the aborted result is gone — it falls into the cached branch and no-ops,
293+
// so the job is enqueued exactly once. (`cleanUpProvingJobState` also drops the promise, which
294+
// was resolved with the abort, so `enqueueJobInternal` below mints a fresh one for the retry.)
295+
this.logger.info(`Reviving aborted proving job id=${job.id} epochNumber=${job.epochNumber}`, {
296+
provingJobId: job.id,
297+
});
298+
this.cleanUpProvingJobState([job.id]);
299+
this.jobsCache.set(job.id, job);
300+
await this.database.deleteProvingJobResult(job.id);
301+
jobStatus = this.#getProvingJobStatus(job.id);
302+
} else {
303+
this.logger.warn(`Cached proving job id=${job.id} epochNumber=${job.epochNumber}. Not enqueuing again`, {
304+
provingJobId: job.id,
305+
});
306+
this.instrumentation.incCachedJobs(job.type);
307+
return jobStatus;
308+
}
284309
}
285310

286311
if (this.isJobStale(job)) {
@@ -306,15 +331,35 @@ export class ProvingBroker implements ProvingJobProducer, ProvingJobConsumer, Pr
306331
}
307332

308333
async #cancelProvingJob(id: ProvingJobId): Promise<void> {
309-
if (!this.jobsCache.has(id)) {
334+
const job = this.jobsCache.get(id);
335+
if (!job) {
310336
this.logger.warn(`Can't cancel a job that doesn't exist id=${id}`, { provingJobId: id });
311337
return;
312338
}
313339

314-
// notify listeners of the cancellation
315-
if (!this.resultsCache.has(id)) {
316-
this.logger.info(`Cancelling job id=${id}`, { provingJobId: id });
317-
await this.#reportProvingJobError(id, 'Aborted', false, undefined, true);
340+
// Leave jobs that have already settled (completed or failed) alone: those results are terminal.
341+
if (this.resultsCache.has(id)) {
342+
return;
343+
}
344+
345+
this.logger.info(`Cancelling job id=${id}`, { provingJobId: id });
346+
this.inProgress.delete(id);
347+
348+
// Record the cancellation as its own settled state and persist it, so it survives a restart and
349+
// notifies the current waiter. Unlike a completion or failure this is not terminal: re-enqueuing
350+
// the same job id revives it (see #enqueueProvingJob), so the abort never permanently blocks the
351+
// proof.
352+
const result: ProvingJobSettledResult = { status: 'aborted' };
353+
this.resultsCache.set(id, result);
354+
this.promises.get(id)?.resolve(result);
355+
this.completedJobNotifications.push(id);
356+
this.instrumentation.incAbortedJobs(job.type);
357+
358+
try {
359+
await this.database.setProvingJobAborted(id);
360+
} catch (saveErr) {
361+
this.logger.error(`Failed to save proving job aborted status id=${id}`, saveErr, { provingJobId: id });
362+
throw saveErr;
318363
}
319364
}
320365

@@ -401,7 +446,6 @@ export class ProvingBroker implements ProvingJobProducer, ProvingJobConsumer, Pr
401446
err: string,
402447
retry = false,
403448
filter?: ProvingJobFilter,
404-
aborted = false,
405449
): Promise<GetProvingJobResponse | undefined> {
406450
const info = this.inProgress.get(id);
407451
const item = this.jobsCache.get(id);
@@ -462,11 +506,7 @@ export class ProvingBroker implements ProvingJobProducer, ProvingJobConsumer, Pr
462506
this.promises.get(id)!.resolve(result);
463507
this.completedJobNotifications.push(id);
464508

465-
if (aborted) {
466-
this.instrumentation.incAbortedJobs(item.type);
467-
} else {
468-
this.instrumentation.incRejectedJobs(item.type);
469-
}
509+
this.instrumentation.incRejectedJobs(item.type);
470510
if (info) {
471511
const duration = this.msTimeSource() - info.startedAt;
472512
this.instrumentation.recordJobDuration(item.type, duration);

yarn-project/prover-client/src/proving_broker/proving_broker_database.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,22 @@ export interface ProvingBrokerDatabase {
3838
*/
3939
setProvingJobError(id: ProvingJobId, err: string): Promise<void>;
4040

41+
/**
42+
* Records that a proof request was cancelled. Unlike a result or error this is not terminal:
43+
* re-enqueuing the same job id revives it, so the aborted state can survive a restart without
44+
* permanently blocking the proof.
45+
* @param id - The ID of the cancelled proof request
46+
*/
47+
setProvingJobAborted(id: ProvingJobId): Promise<void>;
48+
49+
/**
50+
* Clears any stored result for a proof request, returning it to the pending state while keeping
51+
* the job itself. Used when reviving an aborted job so the revival is persisted and a restart
52+
* cannot resurrect the stale aborted state.
53+
* @param id - The ID of the proof request whose result should be cleared
54+
*/
55+
deleteProvingJobResult(id: ProvingJobId): Promise<void>;
56+
4157
/**
4258
* Closes the database
4359
*/

yarn-project/prover-client/src/proving_broker/proving_broker_database/broker_persisted_database.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,28 @@ describe('ProvingBrokerPersistedDatabase', () => {
132132
}
133133
});
134134

135+
it('keeps a queued result-delete ordered after the writes it follows', async () => {
136+
const id = makeRandomProvingJobId(EpochNumber(42));
137+
const job: ProvingJob = {
138+
id,
139+
epochNumber: EpochNumber(42),
140+
type: ProvingRequestType.PARITY_BASE,
141+
inputsUri: makeInputsUri(),
142+
};
143+
await db.addProvingJob(job);
144+
145+
// Abort the job and then delete its result without awaiting the abort first — mimicking a revive
146+
// that re-requests a job whose aborted write may not have flushed yet. Because the delete rides
147+
// the same write queue, it stays ordered after the abort and the result ends up cleared. If it
148+
// were applied out of order the abort would resurrect on reload.
149+
const abortWrite = db.setProvingJobAborted(id);
150+
const deleteWrite = db.deleteProvingJobResult(id);
151+
await Promise.all([abortWrite, deleteWrite]);
152+
153+
const allJobs = await toArray(db.allProvingJobs());
154+
expect(allJobs).toEqual([[job, undefined]]);
155+
});
156+
135157
it('can add items over multiple epochs', async () => {
136158
const numJobs = 5;
137159
const startEpoch = 12;

yarn-project/prover-client/src/proving_broker/proving_broker_database/memory.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@ export class InMemoryBrokerDatabase implements ProvingBrokerDatabase {
3636
return Promise.resolve();
3737
}
3838

39+
setProvingJobAborted(id: ProvingJobId): Promise<void> {
40+
this.results.set(id, { status: 'aborted' });
41+
return Promise.resolve();
42+
}
43+
44+
deleteProvingJobResult(id: ProvingJobId): Promise<void> {
45+
this.results.delete(id);
46+
return Promise.resolve();
47+
}
48+
3949
deleteProvingJobs(ids: ProvingJobId[]): Promise<void> {
4050
for (const id of ids) {
4151
this.jobs.delete(id);

0 commit comments

Comments
 (0)