imbajin commented on code in PR #3308:
URL: https://github.com/apache/hugegraph/pull/3308#discussion_r4243546942
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java:
##########
@@ -564,7 +777,8 @@ public boolean close() {
}
// cancel all running tasks
- for (HugeTask<?> task : this.runningTasks.values()) {
+ for (PendingTask pending : this.runningTasks.values()) {
Review Comment:
Closing now releases an unstarted persistent dispatch only when it has no
cancellation or deletion coordination, without changing its task or durable NEW
status. The identity check also protects concurrent close snapshots.
Regressions verify ordinary and concurrent closes, shared-worker saturation,
and actual durable redispatch by a replacement scheduler.
The latest-head Linux Memory unit, core and API suites passed, including the
durable redispatch and RocksDB reload regressions. The full CI and coverage
checks are also passing.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -2268,40 +2266,77 @@ public void dropGraph(String graphSpace, String name,
boolean clear) {
}
String graphName = spaceGraphName(graphSpace, name);
+ boolean alreadyClosed = g.closed();
+ E.checkState(!clear || !alreadyClosed ||
this.removingGraphs.contains(graphName),
+ "Can't clear an already closed graph '%s' without an
accepted deletion", graphName);
+ // Task results still need the backend until their scheduler has
drained.
+ TaskScheduler scheduler = null;
+ if (clear && !alreadyClosed) {
+ try {
+ // Keep the auth proxy's cached owner and its pre-close ADMIN
check.
+ scheduler = g.taskScheduler();
+ } catch (IllegalStateException e) {
+ if (!(g instanceof StandardHugeGraph) ||
+ TaskManager.instance().getScheduler(g) != null) {
+ throw e;
+ }
+ // A previous plain-graph close removed its scheduler but
retained owners.
+ }
+ }
+ if (clear && scheduler != null && !scheduler.close()) {
+ throw new HugeException("Can't drop graph '%s' while tasks are
active, " +
+ "please retry later", graphName);
+ }
if (clear) {
- this.removingGraphs.add(graphName);
try {
- this.metaManager.removeGraphConfig(graphSpace, name);
+ if (!this.removingGraphs.contains(graphName)) {
+ this.metaManager.removeGraphConfig(graphSpace, name);
+ this.removingGraphs.add(graphName);
+ }
this.metaManager.notifyGraphRemove(graphSpace, name);
} catch (Exception e) {
throw new HugeException(
- "Failed to remove graph config of '%s'", name, e);
+ "Failed to remove graph config of '%s'", e, name);
}
- /**
- * close task scheduler before clear data,
- * because taskinfo stored in backend in
- * {@link org.apache.hugegraph.task.DistributedTaskScheduler}
- */
+ if (!alreadyClosed) {
+ // Unwrapping an auth proxy preserves its ADMIN permission
check.
+ HugeGraph underlying = g.hugegraph();
+ if (underlying instanceof StandardHugeGraph) {
+ ((StandardHugeGraph) underlying).clearBackendForDrop();
+ } else {
+ g.clearBackend();
+ }
+ }
try {
- g.taskScheduler().close();
- } catch (Throwable t) {
- LOG.warn(String.format(
- "Error when close TaskScheduler of %s",
- graphName),
- t);
+ if (!alreadyClosed) {
+ g.close();
+ }
+ } catch (Exception e) {
+ if (!g.closed()) {
+ throw new HugeException("Failed to close graph '%s',
please retry later",
+ e, graphName);
+ }
+ LOG.warn("Graph '{}' closed with an owner cleanup failure",
graphName, e);
}
-
- g.clearBackend();
+ } else {
+ // Remote notifications detach owners only after close succeeds.
try {
g.close();
} catch (Exception e) {
- LOG.warn("Failed to close graph", e);
+ if (!g.closed()) {
+ throw new HugeException("Failed to close graph '%s',
please retry later",
Review Comment:
Reload now retains graphs whose close fails, reopens successfully detached
graphs, and reports the incomplete closure after recovery. It also preserves
backend data and local properties on the non-PD close-only path. Real RocksDB
regressions force a partial close failure and verify registrations, persisted
vertices and configuration before and after an explicit retry.
The latest-head Linux Memory unit, core and API suites passed, including the
durable redispatch and RocksDB reload regressions. The full CI and coverage
checks are also passing.
--
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]
