Copilot commented on code in PR #780:
URL: https://github.com/apache/unomi/pull/780#discussion_r3469109336
##########
services/src/main/java/org/apache/unomi/services/impl/scheduler/SchedulerServiceImpl.java:
##########
@@ -790,6 +790,36 @@ public void preDestroy() {
LOGGER.debug("Error shutting down execution manager: {}",
e.getMessage());
}
+ // Mark tasks still in RUNNING state as CRASHED — they were
interrupted mid-execution.
+ // This allows the next scheduler instance to reschedule them via
CRASHED→SCHEDULED,
+ // and prevents invalid RUNNING→SCHEDULED transitions in shared
persistence environments.
+ // We go directly to persistenceProvider/nonPersistentTasks here
(instead of
+ // getAllTasks()/saveTask()) because shutdownNow is already true at
this point,
+ // and those wrapper methods short-circuit to no-ops once that flag is
set.
+ if (stateManager != null) {
+ try {
+ List<ScheduledTask> tasksToCheck = new
ArrayList<>(nonPersistentTasks.values());
+ if (persistenceProvider != null) {
+ tasksToCheck.addAll(persistenceProvider.getAllTasks());
+ }
Review Comment:
In preDestroy(), this shutdown recovery scans *all* persistent tasks
(`persistenceProvider.getAllTasks()`) and will mark any RUNNING task as
CRASHED. In a clustered deployment, that risks incorrectly crashing tasks
currently running on other nodes when this node shuts down.
To avoid cross-node interference, only consider tasks owned by this node
(e.g., locked by this node via `findTasksByLockOwner(nodeId)`) in addition to
local in-memory tasks.
--
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]
