imbajin commented on code in PR #3308:
URL: https://github.com/apache/hugegraph/pull/3308#discussion_r4253751467
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskManager.java:
##########
@@ -261,7 +251,8 @@ public ServerInfoManager
getServerInfoManager(HugeGraphParams graph) {
}
public void shutdown(long timeout) {
- assert this.schedulers.isEmpty() : this.schedulers.size();
+ E.checkState(this.schedulers.isEmpty(),
Review Comment:
Independent source cross-check confirms the new early guard skips all
executor shutdown/await attempts when a scheduler remains after the final
graph-close attempt. The JVM shutdown hook then loses its graceful-drain
opportunity; a continuing embedded JVM retains live pools. A task merely
running when stop begins is not sufficient if it drains before that final
close, and base shutdown was already conditional pool-by-pool.
One safety constraint on the suggested fix: please do not tear down
supporting executors unconditionally while owners are still retained.
Distributed scheduler close uses its scheduler executor, and pending tasks
still need task-database persistence. A bounded owner drain/close retry
followed by safely ordered executor cleanup and explicit retained-owner failure
preserves that contract. This is source verification, not a new runtime
reproduction.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java:
##########
@@ -1212,12 +1213,16 @@ public void create(String configPath, GlobalMasterInfo
nodeInfo) {
@Override
public void drop() {
+ this.checkGraphNotClosed();
+ TaskScheduler scheduler = this.taskManager.getScheduler(this.params);
+ if (scheduler != null && !scheduler.close()) {
+ // Fence admission before waiting for actual task owners to exit.
+ this.waitUntilAllTasksCompleted();
Review Comment:
The persistent admission fence is real, but this part was already raised in
https://github.com/apache/hugegraph/pull/3308#discussion_r4240715704 and is
consistent with the current retry-only maintenance contract in
docs/server-runtime.md. Earlier feedback also required fencing before returning
with pending work, so automatic reopening would need an explicit abort protocol
for concurrent or partially destructive deletion.
The smallest safe clarification is to state in the refusal error and runtime
guide that new task admission remains closed, deletion can be retried after
owners exit, and resuming ordinary use requires an appropriate close/reopen or
restart procedure. I would track this as the existing recovery-contract
clarification rather than a second newly discovered code defect. No additional
runtime test was performed in this cross-check.
--
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]
