bitflicker64 commented on code in PR #815:
URL:
https://github.com/apache/hugegraph-toolchain/pull/815#discussion_r4267203044
##########
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/executor/LoadContext.java:
##########
@@ -211,6 +215,29 @@ public void updateSchemaCache() {
@SneakyThrows
public synchronized void setLoadingMode() {
String graph = this.client.graph().graph();
+ if (this.loadingOwnerReleased) {
+ throw new LoadException("Loading owner has already been released;
create a new context");
+ }
+ if (this.loadingOwner == null) {
+ try {
+ this.loadingOwner =
this.client.graphs().newLoadingOwner(graph);
+ LOG.info("Preparing LOADING owner {} for graph '{}'; after an
unclean exit, stop related imports " +
+ "and use the Server loading-owner recovery
procedure", this.loadingOwner, graph);
+ } catch (ServerException e) {
+ if (e.status() != 404 || !"HTTP 404 Not
Found".equals(e.getMessage()) ||
+ !("jakarta.ws.rs.NotFoundException".equals(e.exception())
||
+ "javax.ws.rs.NotFoundException".equals(e.exception()) ||
+ "class
jakarta.ws.rs.NotFoundException".equals(e.exception()) ||
+ "class
javax.ws.rs.NotFoundException".equals(e.exception()))) {
+ throw e;
+ }
+ LOG.warn("Server has no loading-owner endpoint; use legacy
mode without concurrent-import protection");
+ }
+ }
+ if (this.loadingOwner != null) {
+ this.client.graphs().acquireLoadingOwner(graph, this.loadingOwner);
Review Comment:
Minor: Hubble never releases this handle. `LoadTask.run()` calls only
`loader.load()`, and `stop()` and `reconnect()` drop the loader without
`shutdown()`, so `unsetLoadingMode()` runs only from the JVM shutdown hook.
On legacy Servers Hubble already left LOADING behind, but an enum PUT to
NONE cleared it. On a #3331 Server each Hubble task and each resume adds an
active owner. The Server then rejects every enum mode change, including
restore, until Hubble exits or an administrator runs `recover`.
Could Hubble call `loader.shutdown()` at the end of `LoadTask.run()` and in
`stop()`, or could the README call out this Hubble limitation next to the
recovery steps?
##########
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/executor/LoadContext.java:
##########
@@ -242,6 +269,18 @@ public void setRestoreMode() {
}
public synchronized void unsetLoadingMode() {
+ if (this.loadingOwner != null) {
+ if (!this.loadingOwnerReleased) {
+ try {
+
this.client.graphs().releaseLoadingOwner(this.client.graph().graph(),
this.loadingOwner);
+ this.loadingOwnerReleased = true;
+ } catch (Exception e) {
+ throw new LoadException("Failed to release LOADING owner
%s; retry the same handle", e,
Review Comment:
Minor: the release-failure message tells the user to "retry the same
handle", but nothing at this head can do that.
`stopThenShutdown()` calls `context.close()` right after this throws, which
closes the clients and sets `closed`, so a second `shutdown()` returns early.
`LoadingModeOwner.toString()` no longer includes the token, so this message and
the `Preparing LOADING owner` log give only `instance/epoch`. README line 198
says the opposite: another `shutdown()` does not retry, so use administrator
recovery.
Point the message at the Server recovery procedure from the README instead
of a retry.
--
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]
