Re: [PR] fix(server): fix the scheduler and the scheduler selection logic [hugegraph]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #2937:
URL: https://github.com/apache/hugegraph/pull/2937#discussion_r2969209968


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -1960,7 +1972,7 @@ public HugeGraph graph(String graphSpace, String name) {
         } else if (graph instanceof HugeGraph) {
             return (HugeGraph) graph;
         }
-        throw new NotSupportException("graph instance of %s", graph.getClass());
+        throw new NotFoundException(String.format("Graph '%s' does not exist", name));
     }

Review Comment:
   `graph()` now throws NotFoundException even when a graph entry exists but is not an instance of `HugeGraph` (the prior behavior threw NotSupportException). This can mask configuration/initialization problems as a 404. Consider restoring the previous exception type for the non-`HugeGraph` case, and only throwing NotFound when `graph == null`/missing.



##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java:
##########
@@ -353,6 +396,18 @@ public boolean close() {
             cronFuture.cancel(false);
         }
 
+        // Wait for cron task to complete to ensure all transactions are closed
+        try {
+            cronFuture.get(schedulePeriod + 5, TimeUnit.SECONDS);
+        } catch (CancellationException e) {
+            // Task was cancelled, this is expected
+            LOG.debug("Cron task was cancelled");
+        } catch (TimeoutException e) {
+            LOG.warn("Cron task did not complete in time when closing scheduler");
+        } catch (ExecutionException | InterruptedException e) {
+            LOG.warn("Exception while waiting for cron task to complete", e);
+        }

Review Comment:
   `cronFuture.get(...)` will not reliably wait for an in-flight `scheduleWithFixedDelay` execution to finish once the future is cancelled (it typically throws `CancellationException` immediately). That undermines the goal of ensuring cron scheduling has stopped before closing transactions. Consider using a barrier on the single-threaded `schedulerExecutor` (e.g., submit a no-op and wait) or shutting down/awaiting the executor, and ensure `InterruptedException` restores the interrupt flag when caught.



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org