jstastny-cz commented on code in PR #2301:
URL:
https://github.com/apache/incubator-kie-kogito-apps/pull/2301#discussion_r2811805392
##########
jobs/jobs-common-embedded/src/main/java/org/kie/kogito/app/jobs/impl/VertxJobScheduler.java:
##########
@@ -370,7 +456,7 @@ public void cancel(String jobId) {
JobDetails jobDetails = jobStore.find(jobContext, jobId);
Review Comment:
I attempted at altering the logic, but related to this, the topic of
periodic timers came to my attention - even though job is in `RUNNING`, it can
be a periodic timer job which would mean if `cancel` becomes no-op, the future
scheduled job (once the current moves `RUNNING` -> `EXECUTED` -> `SCHEDULED`
via `computeAndScheduleNextJobIfAny` method) would not be affected by the
cancel call.
Though I am inclined to say we already are not canceling everything ...
So if we consider store operations wise:
1. job is `RUNNING`
2. someone call cancel - > removes the entity from job store
3. Eventually job tries to move to `EXECUTED` using store update operation
(uses merge)
4. It basically re-creates the entity in store.
5. in case of periodic timers, that one is picked up again when its time
comes.
To me it sounds like we should either wait for the timer to move to some
inactive state (I'd say `SCHEDULED` or `RETRY`) before performing the cancel
operation, or perhaps even throw an exception when the job being canceled is
not in `SCHEDULED` or `RETRY` (i.e. is not waiting for execution). WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
