Delete scheduled jobs in one DeleteJob call - #385
Open
scott-rc wants to merge 3 commits into
Open
Conversation
DeleteJob refused every non-terminal job with an error claiming it was "running or has pending requests", which is false for a Scheduled job and forced purge scripts to pair a CancelJob with every DeleteJob. The delete path now accepts Scheduled jobs directly: a serializable-snapshot transaction re-reads the status as its first operation (the outer read only routes) and performs the same cleanup cancellation does -- pending task removal with the time-0 fallback, concurrency holder and request release with post-commit slot regrant and broker-buffer eviction, requester-counter decrement, index cleanup, and counter bookkeeping without the completed-jobs decrement that only terminal jobs carry. The in-memory queue gauge is decremented once per deleted job, with rollback on failed commit attempts so conflict retries cannot drift it. Running jobs stay refused, and JobInProgress now carries the job's status so the message names what the job is actually doing. The cannot-delete-scheduled tests are repurposed as positive coverage, with the double-duty pending-request test split so its Running-refusal coverage survives.
The one-call scheduled delete and the status-naming Running refusal are operator-facing guarantees, so they get end-to-end coverage through the RPC layer: a future-dated scheduled job deletes with a single DeleteJob call and GetJob then returns NotFound, and deleting a leased job surfaces an error message that names the Running status. No proto or siloctl changes -- the CLI's single-ID delete now works on scheduled jobs as-is.
Attempt rows carry no TTL until a job reaches a terminal status, so deleting a Scheduled job that was waiting on a retry orphaned its prior attempt rows forever and let them resurface under a re-enqueued job with the same id; the delete transaction now sweeps the job's attempt prefix. The pending task, holder, and request cleanup that scheduled deletion duplicated from cancellation moves into a shared helper so the two paths cannot silently diverge, and the request-record scan inside it now reads through the transaction so a concurrent grant conflicts instead of racing the cleanup. Also: delete_job gains the shard-scoped tracing span its sibling cancel_job carries; the pending-request test reads the requester counter through the public accessor (its raw read decoded big-endian against a little-endian encoding); the running-refusal gRPC test pins the status code; the one-call-delete gRPC test pins that no leasable task survives; and grant-dependent dequeues in the split refusal test use the bounded retry loop.
Contributor
Author
|
This change is part of the following stack: Change managed by git-spice. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claude wanted to add this, but I'm guessing this breaks the alloy spec so you can just ignore this PR if you don't want this.
Purging scheduled jobs previously required two RPCs per job:
DeleteJobrefused every non-terminal status withcannot delete job <id>: job is currently running or has pending requests-- a message that is false for aScheduledjob -- so operators had to send aCancelJobfirst to make each job deletable. A staging fixture cleanup that purged 10,000 inert future-dated jobs paid that cost per job, and the misleading error text made the workaround undiscoverable.DeleteJobnow deletesScheduledjobs directly. The scheduled branch runs inside the same serializable-snapshot transaction machinery cancellation uses, re-reads the job status as its first in-transaction operation (the pre-transaction read only routes), and performs the full cleanup a cancel-then-delete pair produces: pending task removal (with the time-0 fallback lookup), concurrency holder and request-record release with post-commit slot regrant and broker-buffer eviction, requester-counter decrement, status-time/metadata index cleanup, attempt-row sweep, and counter bookkeeping without the completed-jobs decrement that only terminal jobs carry. The in-memory queue gauge decrements exactly once per deleted job, with rollback on failed commit attempts so conflict retries cannot drift it. That cleanup logic is shared with the cancel path via one helper so the two cannot silently diverge.Runningjobs stay refused, andJobInProgressnow carries the job's status so the error names what the job is actually doing (job is Running; cancel it or wait for it to finish). Terminal-status deletes and idempotent missing-job deletes are unchanged. No proto or siloctl changes -- the CLI's single-ID delete works on scheduled jobs as-is.Review notes: the scheduled-refusal tests are repurposed as positive coverage (the double-duty pending-request test is split so its Running-refusal coverage survives); counter exactness is asserted mid-purge as well as at the end, since the queue gauge floors at zero and only a partial-purge checkpoint can catch a double decrement; gRPC tests pin the one-call delete (GetJob NotFound, no leasable task) and the status-naming refusal end-to-end.