Repository navigation
refactor(work): run the attempt chain and mutations as writer jobs - #4158
Conversation
StartAttempt, Synthesize, RetryAttempt and ResumeAttempts now run their contracts services inside one ExactSqlHandle::transact job over WorkSqliteJobStorage, and the awaiting task parks no native thread. StartAttempt's run reservation fence used to run in its own transaction before the admission transaction, so a pause committed in between surfaced as an identity conflict from the insert's fence; the fence, the replay check, the lease epoch, the product link and the attempt row now commit together, and a racing pause either lands first and is reported as the paused conflict or lands after the attempt. The current product graph the admissions read is replayed from the journal before the job, on the blocking pool, and handed to the job as a PrefetchedWorkGraphReadV1 that answers only that exact read; the link's graph-version compare-and-swap is its staleness fence. Synthesis hydrates its source payloads through the job storage, because the workflow artifact store's own transaction would wait on the writer running the job. The workflow rebind a retry carries is still read before the job and appended under its sequence check inside it. The attempt, effect-holder, owner-authorization and admission ports drop their Send + Sync bound so the job storage can implement them, sharing every SQL body with the handle storage through RegisteredWorkWrite. The reservation read moves to a WorkRunReservationPort. admit_retry and admit_synthesis move into the retry and synthesis storage ports; their handle implementations, and the handle retry and synthesis storage, had no other production caller and are deleted. Resume now fences every open attempt or none: a refusal partway through rolls back the attempts it had already fenced.
The provider-attempt driver now awaits each durable step between process events as its own writer job: the effect dispatch record and settlement, mark_running, the provider-unavailable fence, the recovery failure, the cancellation acknowledgement and escalation, and the terminal settlement. Process waits stay on the attempt task. The driver reaches these steps through an AttemptStepsV1 trait; the registered Work services implement it with job-backed async methods, and the driver tests answer it from the in-memory services. The attempt task still runs on a blocking-pool thread. It persists provider output through the synchronous workflow artifact store and reconciles the workflow fan-out after the attempt, and both still park their thread on the writer. The effect-holder read moves to a WorkAttemptEffectReadPortV1 that retry safety, execution history and the leak evidence use. The handle storage implements only that read; its dispatch and settlement writes had no other production caller and are deleted, as is the sync attempt_effects service accessor.
CancelAttempt now runs the contracts cancellation request inside one ExactSqlHandle::transact job over WorkSqliteJobStorage: the load, the replay or conflict check and the fenced update commit together, and the awaiting task parks no native thread. The in-process signal to a live provider task still follows the committed request. The handle-based request_cancellation keeps one production caller, the workflow fan-out fail-fast path, which is still synchronous.
Create, ReviewProposal, AcceptProposal, AdmitExecution and MutateGraph now run the contracts product mutation service inside one ExactSqlHandle::transact job over WorkSqliteJobStorage: the revision check, the command replay check and the event append with its verified version commit together, and the awaiting task parks no native thread. They all route through one RegisteredWorkProductServicesV1::mutate. The current graph a mutation checks its expected version against is replayed from the journal before the job, on the blocking pool, and handed to the job as a PrefetchedWorkGraphReadV1; the append's graph-version compare-and-swap remains the staleness fence. AdmitExecution still prepares its execution snapshot from a reader before the job. WorkProductEventPortV1 drops its Send + Sync bound so the job storage can implement it, sharing the replay and append bodies with the handle storage. The handle append keeps production callers in the workflow fan-out, which is still synchronous.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| .transact_with_graph(graph, move |storage| { | ||
| tracedecay_contracts::WorkRunControlService::new(storage).admit_reservation( | ||
| &context, | ||
| &command.task_id, | ||
| &command.run_id, | ||
| )?; | ||
| tracedecay_contracts::WorkProductAttemptServiceV1::new(storage) |
There was a problem hiding this comment.
🟡 Workflow children retain split reservation fence
When a pause races a workflow child, start_attempt is bypassed by admit_workflow_child. Its separate reservation check can pass before the pause, then insertion fails and fan-out reconciliation reports unavailability.
Learn more
The workflow fan-out scheduler is another production caller of Work attempt admission. It checks the run reservation in admit_workflow_child, then separately invokes start_against_registered_topology. A pause can commit between those calls. The insertion checks the paused row, but map_attempt_error turns that refusal into an identity conflict; the scheduler maps the result to unavailable. The same scheduler also retains direct synchronous product mutation calls. This leaves a production caller outside the Work cutover and the race the new job removes.
Example: A child passes its reservation check while its run is running. An operator pauses that run before the child inserts its attempt. Reconciliation returns workflow-unavailable instead of an admitted child or a paused refusal.
Recommended fix: Migrate the workflow child admission to RegisteredWorkProductServicesV1::start_attempt, with its run fence inside the writer job. Move the workflow child product writes to mutate in the same cutover, adapting the scheduler's async call chain and preserving its typed pause refusal.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ) | ||
| }) | ||
| .await | ||
| .unwrap_or_else(|_| Err(attempt_unavailable())) |
There was a problem hiding this comment.
🟡 Attempt commit uncertainty becomes generic unavailability
If a writer commit fails, start_attempt reports unavailable rather than an uncertain commit. The former admit_attempt distinguished this outcome; synthesis and retry now lose that distinction too.
Learn more
transact_with_graph returns an outer ExactSqlError if the writer job's transaction cannot commit, even when the service already returned an attempt. The new mapping converts every outer error into WorkAttemptStorageError::Unavailable. The old admit_attempt used DurabilityUncertain specifically for commit failures, which product_admission_problem exposes as an identifiable diagnostic. Synthesis and retry use the same new outer-error mapping, so all three admissions lose this signal.
Example: SQLite reports an error while committing a newly inserted attempt. The caller receives application.work-attempt.storage-unavailable rather than application.work-attempt.product-durability-uncertain, although the durable outcome cannot be determined from that error.
Recommended fix: Preserve the transaction phase or classify commit errors in ExactSqlHandle::transact, then map commit failures to the attempt durability-uncertain problem for start, synthesis, and retry. Keep pre-commit failures mapped to unavailable.
Was this helpful? React with 👍 or 👎 to provide feedback.
| crate::native_runtime::spawn_blocking(move || read(&storage)) | ||
| .await | ||
| .unwrap_or_default() |
There was a problem hiding this comment.
🟡 Failed graph prefetch silently becomes empty
When the blocking read task fails, prefetch_graph substitutes an empty graph without reporting the failure. The writer then returns a generic graph-read error instead of the prefetch failure.
Learn more
spawn_blocking returns an error when the blocking task fails or cannot return its result. This branch silently replaces that error with a default PrefetchedWorkGraphReadV1, whose read_graph refuses every read as unavailable. The normal application service thus reports an unrelated graph-read failure and loses the actual task failure. The repository requires production failures to stay typed and prohibits silent fallbacks.
Example: A blocking prefetch task panics. A mutation's writer job receives an empty prefetch and reports EventAuthorityUnavailable, rather than exposing the failed blocking task.
Recommended fix: Return a typed error from prefetch_graph and propagate it through start_attempt, mutate, synthesize, and retry_attempt without submitting a writer job on prefetch-task failure.
Was this helpful? React with 👍 or 👎 to provide feedback.
Step 5 of the storage redesign (
docs/plans/storage/README.md): the Work attempt chain and the Work product mutations run as writer jobs throughExactSqlHandle::transact(#4133), so the awaiting task parks no native thread.AttemptStepsV1trait.RegisteredWorkProductServicesV1::mutatejob; the append's graph-version compare-and-swap stays the staleness fence.The current graph is replayed from the journal before each job and handed in as a
PrefetchedWorkGraphReadV1that answers only that exact read. Handle-based writes with no remaining production caller are deleted.Still synchronous, for the next step-5 slice: the workflow artifact store and the workflow fan-out (including its fail-fast
request_cancellationand handle appends), so the attempt task still runs on a blocking-pool thread.Test plan
bazel build --config=clippyon the touched cratesrustfmt_check_2024on the touched crates