imbajin commented on code in PR #3308:
URL: https://github.com/apache/hugegraph/pull/3308#discussion_r4250613665
##########
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:
The earlier reload recovery fix has been intentionally removed to keep this
PR focused on local drain and deletion. The close failure remains visible and
an open graph remains registered; full reload recovery is deferred rather than
claimed fixed by this version. The runtime contract now requires restart for
configuration changes, and the broader website documentation is held as a draft
pending reconciliation.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java:
##########
@@ -469,17 +550,200 @@ private void saveTaskWithoutResult(HugeTask<?> task) {
});
}
+ private boolean cancelLocal(PendingTask pending, Consumer<Boolean>
afterCancel) {
+ return this.cancelLocal(pending, null, afterCancel);
+ }
+
+ private boolean cancelLocal(PendingTask pending, Runnable beforeCancel,
+ Consumer<Boolean> afterCancel) {
+ return this.cancelLocal(pending, beforeCancel, afterCancel, false);
+ }
+
+ private boolean cancelLocal(PendingTask pending, Runnable beforeCancel,
+ Consumer<Boolean> afterCancel, boolean
terminalOnly) {
+ if (SCHEDULER_WORKER.get() == this.schedulerExecutor) {
+ return this.cancelOwned(pending, beforeCancel, afterCancel,
terminalOnly);
+ }
+ // Keep callbacks and final persistence off the request/close thread
and
+ // the DB worker, because callbacks may synchronously save to that
worker.
+ return this.call(() -> {
+ SCHEDULER_WORKER.set(this.schedulerExecutor);
+ try {
+ return this.cancelOwned(pending, beforeCancel, afterCancel,
terminalOnly);
+ } finally {
+ SCHEDULER_WORKER.remove();
+ // An interrupted request may stop waiting before this owner
finishes.
+ this.finishCancelledQueued(pending);
+ }
+ }, this.schedulerExecutor, true);
+ }
+
+ private boolean cancelOwned(PendingTask pending, Runnable beforeCancel,
+ Consumer<Boolean> afterCancel, boolean
terminalOnly) {
+ LockResult acquired = null;
+ boolean acquire;
+ synchronized (pending) {
+ if (this.runningTasks.get(pending.task.id()) != pending) {
+ return false;
+ }
+ acquire = !pending.task.ephemeralTask() && pending.taskLock ==
null;
+ pending.cancellationDepth++;
+ }
+ boolean entered = false;
+ boolean coordinated = false;
+ try {
+ if (acquire) {
+ // Don't block publication/release of this runner's token
during the lock RPC.
+ acquired = this.tryLockTask(pending.task.id().asString());
+ if (!acquired.lockSuccess()) {
+ return false;
+ }
+ synchronized (pending) {
+ pending.taskLock = acquired;
+ }
+ }
+ entered = true;
+ if (!pending.task.ephemeralTask()) {
+ HugeTask<?> stored;
+ try {
+ stored = this.taskWithoutResult(pending.task.id());
+ } catch (NotFoundException e) {
+ pending.discardQueued = true;
+ coordinated = true;
+ return false;
+ }
+ if (beforeCancel == null) {
+ if
(TaskStatus.COMPLETED_STATUSES.contains(stored.status())) {
+ pending.discardQueued = true;
+ coordinated = true;
+ return false;
+ }
+ if (terminalOnly) {
+ if (stored.status() == TaskStatus.DELETING) {
+ pending.discardQueued = true;
+ coordinated = true;
+ } else if (pending.task.isCancelled() &&
+ pending.task.status() ==
TaskStatus.CANCELLED &&
+ stored.status() == TaskStatus.CANCELLING) {
Review Comment:
The broad cancellation-persistence replay protocol has been deferred with
task-record recovery. This version guarantees that physical execution, accepted
local callbacks and native cleanup have finished before graph deletion; it does
not guarantee recovery of every final task-status publication failure.
Failed/uncertain scheduler cleanup retains the scheduler and graph instead of
acknowledging deletion. Such storage/recovery failures require explicit
maintenance, as documented.
--
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]
