Implement pausing for submissions#132
Conversation
a9f5556 to
bcd4161
Compare
|
TODO: How to deal with reserved chunks when pausing? |
b09f93b to
7e7052d
Compare
Discussed this with @ReinierMaas yesterday, we tried to make it work s.t. chunk completions for cancelled and paused submissions would still be processed. However, implementing that proved to be difficult:
After struggling with it for a while, I thought it would be easier to invoke the idempotency assumption we have for how consumers process chunks, and just ignore the completion in case the submission is cancelled or paused. This results in less edge-cases and simpler code in the end, at the cost of slightly higher compute cost. But I think it's worth the trade-off: there are enough edge-cases in the codebase as it is :). @ReinierMaas, let me know if you disagree ;). |
990a5d2 to
71ace4e
Compare
| pass | ||
|
|
||
|
|
||
| class ChunkNotFoundError(IncorrectUsageError): |
There was a problem hiding this comment.
Note that ChunkNotFoundError can be dropped since it is (and was) dead code.
| ) | ||
| .await | ||
| }) | ||
| .await; |
There was a problem hiding this comment.
Note that we were silently dropping errors here before, which went unnoticed because of the underscore prefix of _chunk_size.
21ce8a7 to
9ee94b3
Compare
u63 forces us to wrap literals in `u63::new`, and we need to convert to u64 at actual usage sites anyway.
354327e to
e39c8a9
Compare
Introduce `submissions_paused` and `chunks_paused` tables
(alongside the existing `submissions_{completed,failed,cancelled}` and
`chunks_{completed,failed}` tables).
Pausing a submission atomically moves it from `submissions` →
`submissions_paused` and its remaining chunks from `chunks` →
`chunks_paused`. Because paused chunks are no longer in the `chunks`
table, the consumer dispatcher naturally skips them without any changes
to the dispatch query.
Unpausing reverses the move and notifies waiting consumers.
Paused submissions can also be inserted directly in the paused state
(via the new `paused: bool` field on `InsertSubmission`); in that
case `notify_on_insert` is intentionally not fired.
Paused submissions are cancellable; `cancel_submission` now handles
the case where the submission is found in `submissions_paused`.
Also, make complete_chunk and fail_chunk not error when processing
previously completed, failed, or paused chunks.
e39c8a9 to
82397bd
Compare
Thought some more: if the assumption is that a paused job should not have any work being done, this will violate that. Maybe easier to just only allow pausing of not yet started submissions for now? Other option is to make pausing async, and only transition from |
1 participant
Introduce
submissions_pausedandchunks_pausedtables (alongside the existingsubmissions_{completed,failed,cancelled}andchunks_{completed,failed}tables).Pausing a submission atomically moves it from
submissions→submissions_pausedand its remaining chunks fromchunks→chunks_paused. Because paused chunks are no longer in thechunkstable, the consumer dispatcher naturally skips them without any changes to the dispatch query.Unpausing reverses the move and notifies waiting consumers.
Paused submissions can also be inserted directly in the paused state (via the new
paused: boolfield onInsertSubmission); in that casenotify_on_insertis intentionally not fired.Paused submissions are cancellable;
cancel_submissionnow handles the case where the submission is found insubmissions_paused.