Graylog2 / Graylog2/graylog2-server
System job cancel is silently lost when queued or rescheduled (follow-up to #27080)
- Dominant language
- Java
- Stars
- 8.1k
- Forks
- 1.1k
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 217
Description
## Summary
Follow-up to #27080. Two edge cases in the shared job scheduler still cause a cancel to be silently lost. Both live in `DBJobTriggerService`, which is shared by the user job scheduler (`scheduler_triggers`) and the system job scheduler (`scheduler_system_triggers`), so they affect both and warrant their own change and review, separate from the user-facing fix in #27080.
**Severity: low.** Both cases require a cancel to arrive inside a narrow window. (a) is wasted work (job starts then immediately stops), not a correctness failure. (b) loses a cancel only if it lands during the brief reschedule/retry-backoff window. Confirmed against current code, but not user-visible in the common path.
## Current Behavior
**(a) Cancelling a queued/runnable trigger lets the job start and then stop again.**
`DBJobTriggerService#cancelTriggerByQuery` sets `is_cancelled=true` on whatever trigger the query matches, regardless of status. `nextRunnableTrigger` locks the trigger and moves it `RUNNABLE -> RUNNING` but never clears `is_cancelled`, so a trigger cancelled while queued transitions to RUNNING with the flag still set. The job starts, observes `ctx.isCancelled()`, and stops. Wasted work rather than a clean "never started".
**(b) `releaseTrigger` clears the cancel flag on every release, so a cancel is lost across a reschedule/retry.**
`releaseTrigger` unconditionally does `set(is_cancelled, false)` on release. When a job returns a `JobTriggerUpdate` with a next time (retry/reschedule), the trigger returns to RUNNABLE with `is_cancelled=false`, so a cancel that arrived during the just-finished execution is wiped and never observed by the next run.
This second case is reachable by the job in #27080: `RebuildIndexRangesJob` reschedules with `SystemJobResult.withRetry(Duration.ofSeconds(5), Integer.MAX_VALUE)` on `AlreadyLockedException`. A cancel issued during that 5s retry-backoff window is lost.
## Possible Solution
- **(a)** Don't start an already-cancelled trigger: in `nextRunnableTrigger`, skip/complete a trigger whose `is_cancelled` is already true instead of running it.
- **(b)** On `releaseTrigger`, if the trigger was cancelled while a reschedule is pending, honor the cancel (complete the trigger) rather than resetting the flag and rescheduling.
Both changes touch the scheduler core shared by user and system jobs, so they need test coverage for both schedulers and careful review of the user-job retry paths. Consider whether this should be backported (the core #27080 fix targets 7.1; this hardening may not need to follow).
## Gaps in the proposed solution (must resolve before implementing)
The two solutions above are not safe as written, because the shared `DBJobTriggerService` hosts **both one-shot and recurring** triggers: `OnceJobSchedule` (system jobs, archive `ArchivingJobHandler`, datalake) **and** `CronJobSchedule` / `IntervalJobSchedule` (event definitions, etc.).
1. **(b) "complete the trigger" is correct for `Once` schedules but wrong for recurring ones.** Completing a cron/interval trigger tears down the *entire schedule*, not just the current occurrence — a worse regression than the low-severity bug it fixes.
2. **The data model cannot distinguish a retry-reschedule from a normal recurring reschedule.** `JobTriggerUpdate` carries only `nextTime` / `data` / `status` / `concurrencyReschedule`; both a retry (`withRetry`) and a cron re-fire are just `withNextTime(...)`. So there is no mechanical signal to apply the "honor cancel" behavior only where it is safe.
3. **(a) needs a defined completion path.** Merely excluding `is_cancelled=true` from the `nextRunnableTrigger` filter would strand a cancelled RUNNABLE trigger (never runs, never completes). "Complete it" is right for `Once`; for a recurring trigger the correct behavior is *skip this occurrence and reschedule to the next fire time* while clearing the flag — again requiring schedule-type awareness.
4. **The cancel flag is unscoped, so precise attribution is racy.** `is_cancelled` is a single live boolean with no execution/lock generation token. Any attempt to attribute a cancel to "this execution" vs. "the reschedule window" is racy across threads/nodes. A clean fix likely needs to scope the cancel to a lock/execution generation (e.g. the `triggeredAt` / lock it targeted) rather than a plain boolean.
5. **Cancel semantics are undefined.** "Cancel a running job" — does it mean *this occurrence* or *the whole recurring schedule*? The fix must decide. **Recommended safe scoping: apply the new completion behavior only to non-recurring (`Once`) schedules** — this covers the actual reported scenario (system jobs, archive, datalake are all `Once`) while leaving recurring triggers with today's "cancel this run, next run proceeds" semantics.
6. **`forceReleaseOwnedTriggers` also resets `is_cancelled=false`.** Consistent with release today; revisit it if (b)'s semantics change.
7. **Test coverage.** The existing `cancelTriggerByQuery` test only asserts the flag is set. New tests must cover both schedulers **and** both schedule types (`Once` vs. `Cron`/`Interval`), specifically proving recurring schedules are not completed by a cancel.
## Context
Split out from #27080 to keep the user-facing bugfix (missing REST cancel path) isolated from behavior changes in the shared scheduler.
Relevant code (`graylog2-server/src/main/java/org/graylog/scheduler/`):
- `DBJobTriggerService#cancelTriggerByQuery`, `#nextRunnableTrigger`, `#releaseTrigger`
- `JobExecutionContext#isCancelled` (reads the persisted `is_cancelled` flag live)
- `RebuildIndexRangesJob#doExecute` (the retry path)
## Checklist
- [ ] This issue fix need to be backported.
- [ ] Does this issue have **security** implications?
Contributor guide
Research direction
Start by reading DBJobTriggerService#cancelTriggerByQuery, #nextRunnableTrigger, #releaseTrigger, and forceReleaseOwnedTriggers, then inspect the existing cancelTriggerByQuery test. Trace JobExecutionContext#isCancelled and RebuildIndexRangesJob#doExecute before defining tests for both schedulers and Once versus Cron/Interval schedules. Done means cancellation is not silently lost while recurring schedules remain intact.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, testing
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100