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: [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]
