VGalaxies commented on code in PR #2937:
URL: https://github.com/apache/hugegraph/pull/2937#discussion_r3469654413
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java:
##########
@@ -284,14 +295,41 @@ protected <V> void initTaskParams(HugeTask<V> task) {
}
}
+ /**
+ * Note: This method will update the status of the input task.
+ *
+ * @param task
+ * @param <V>
+ */
@Override
public <V> void cancel(HugeTask<V> task) {
- // Update status to CANCELLING
- if (!task.completed()) {
- // Task not completed, can only execute status not CANCELLING
- this.updateStatus(task.id(), null, TaskStatus.CANCELLING);
+ E.checkArgumentNotNull(task, "Task can't be null");
+
+ if (task.completed() || task.cancelling()) {
+ return;
+ }
+
+ LOG.info("Cancel task '{}' in status {}", task.id(), task.status());
+
+ // Check if task is running locally, cancel it directly if so
+ HugeTask<?> runningTask = this.runningTasks.get(task.id());
+ if (runningTask != null) {
+ boolean cancelled = runningTask.cancel(true);
+ if (cancelled) {
+ task.overwriteStatus(TaskStatus.CANCELLED);
+ }
+ LOG.info("Cancel local running task '{}' result: {}", task.id(),
cancelled);
+ return;
+ }
+
+ // Task not running locally, update status to CANCELLING
+ // for cronSchedule() or other nodes to handle
+ TaskStatus currentStatus = task.status();
Review Comment:
**High: Distributed cancellation can be lost on status races**
`hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java:327`
**Evidence**
- `cancel()` snapshots `task.status()` from the caller object and passes it
as `prestatus`; `updateStatus()` then reloads storage and rejects the update if
the stored status no longer equals that stale value at lines 523-543.
**Impact**
- If a remote task moves from `NEW` to `RUNNING` between API read and
cancellation, the cancel request is dropped and the task keeps running.
**Requested fix**
- Re-read/retry in the cancellation update path and transition any stored
non-completed, non-cancelling status to `CANCELLING`.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java:
##########
@@ -1632,7 +1627,9 @@ public <T> void submitEphemeralJob(EphemeralJob<T> job) {
@Override
public String schedulerType() {
- return StandardHugeGraph.this.schedulerType;
+ // Use distributed scheduler for hstore backend, otherwise use
local
+ // After the merger of rocksdb and hstore, consider whether to
change this logic
+ return StandardHugeGraph.this.isHstore() ? "distributed" : "local";
Review Comment:
**High: Non-hstore clustered tasks now use uncoordinated local scheduling**
`hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java:1632`
**Evidence**
- `schedulerType()` now returns `"local"` for every non-hstore backend, and
local scheduling restores every pending task without server filtering at
`StandardTaskScheduler.java:149-156` and immediately saves/submits all normal
tasks at `StandardTaskScheduler.java:209-211`. The removed code used configured
scheduler type, server ownership, master/worker scheduling, and
`HugeServerInfo.suitableFor()` role matching.
**Impact**
- Non-hstore deployments sharing a backend can execute the same persisted
task on multiple servers, and computer jobs can run on the submitting
non-computer server instead of a `server.role=computer` worker.
**Requested fix**
- Keep uncoordinated local scheduling only for truly single-process stores;
preserve a configurable/cluster-safe scheduler path with ownership and
computer-role routing for shared or multi-server deployments.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/StandardTaskScheduler.java:
##########
@@ -146,19 +143,17 @@ private TaskTransaction tx() {
@Override
public <V> void restoreTasks() {
- Id selfServer = this.serverManager().selfNodeId();
List<HugeTask<V>> taskList = new ArrayList<>();
// Restore 'RESTORING', 'RUNNING' and 'QUEUED' tasks in order.
+ // Single-node mode: restore all pending tasks without server filtering
for (TaskStatus status : TaskStatus.PENDING_STATUSES) {
Review Comment:
**Medium: Legacy `SCHEDULING` and `SCHEDULED` tasks are orphaned after
upgrade**
`hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/StandardTaskScheduler.java:149`
**Evidence**
- Restore only scans `TaskStatus.PENDING_STATUSES`, which is `RESTORING`,
`RUNNING`, and `QUEUED`; `SCHEDULING`/`SCHEDULED` remain enum values but the PR
removed the periodic master/worker loop that consumed them.
**Impact**
- Tasks persisted in those states before upgrade can remain permanently
stuck and never execute or finish.
**Requested fix**
- Add startup migration/drain logic for `SCHEDULING` and `SCHEDULED` tasks,
with a regression test using persisted legacy task states.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -1635,23 +1645,14 @@ private void checkBackendVersionOrExit(HugeConfig
config) {
}
private void initNodeRole() {
- String id = config.get(ServerOptions.SERVER_ID);
+ boolean enableRoleElection = config.get(
+ ServerOptions.ENABLE_SERVER_ROLE_ELECTION);
+ E.checkArgument(!enableRoleElection,
Review Comment:
**Medium: Existing role-election configs now fail startup**
`hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:1650`
**Evidence**
- `server.role_election` remains a public documented option in
`ServerOptions.java:45-51`, but `initNodeRole()` rejects `true` with `"The
server.role_election is no longer supported"`; the later election startup
branch still checks the same option at lines 1665-1667.
**Impact**
- Existing deployments with `server.role_election=true` fail during
`GraphManager` construction after upgrade.
**Requested fix**
- Preserve backward-compatible startup behavior, or remove/deprecate the
option with an explicit migration path and tests instead of leaving
supported-looking config that aborts startup.
--
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]
