imbajin commented on code in PR #3308:
URL: https://github.com/apache/hugegraph/pull/3308#discussion_r4250798813
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java:
##########
@@ -120,7 +125,7 @@ private static boolean sleep(long ms) {
}
}
- public void cronSchedule() {
+ public synchronized void cronSchedule() {
Review Comment:
[P2] Keep the close fence independent of long-running cron work
Making the entire cronSchedule() method synchronized also puts its database
queries and synchronous cancellation callbacks under the scheduler monitor.
close() now needs that same monitor before setting closed, so if an in-flight
cron is blocked in storage or a callback, close() blocks at monitor acquisition
and never reaches the existing timed cron barrier. Previously it could fence
admission and return false when that barrier timed out. This can leave graph
deletion/shutdown waiting indefinitely instead of returning a retryable refusal.
Please keep admission coordination narrow, without holding this monitor
across cron I/O/callbacks, and add a test that blocks a real cron operation and
verifies close can still fence admission and reach its timeout path.
This was independently cross-checked against base 68fb045 from source; no
new runtime reproduction was performed. The current timeout tests mock
waitUntilAllTasksCompleted and do not exercise this monitor-contention path.
--
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]
