bitflicker64 commented on code in PR #525:
URL: https://github.com/apache/hugegraph-doc/pull/525#discussion_r4277070683


##########
content/en/docs/quickstart/toolchain/hugegraph-loader.md:
##########
@@ -986,6 +989,38 @@ bin/hugegraph-loader.sh \
 
 Expected result: two vertices and zero edges imported. This example uses 
dedicated schema names to avoid conflicts with a preloaded `person` label. In 
Hubble, run `g.V().hasLabel('docs_smoke_person').values('docs_name')` to 
confirm `docs-smoke-alice` and `docs-smoke-bob`.
 
+#### 3.4.6 Concurrent imports and LOADING ownership {#loading-ownership}
+
+Starting with HugeGraph 1.8.0, Loader supports concurrent import protection 
via the LOADING ownership protocol.
+
+```mermaid
+graph TD
+    A["Graph Mode: NONE"] -->|First Loader acquires owner| B["LOADING Mode 
(Acquires Token)"]
+    B -->|Concurrent Loaders join| B
+    B -->|Loader finishes & releases| B
+    B -->|Last Loader releases| C["Restores to NONE"]
+    B -.->|Abnormal exit / kill -9| D["Manual REST API Release / Restart"]
+    D -.-> A
+```
+
+1. **Probe and Acquire**: Loader probes `/mode/owners`. The first Loader 
obtains an ownership handle (`instance/epoch/token`),
+   transitioning graph mode from `NONE` to `LOADING`; concurrent Loaders can 
join.
+2. **Data Import**: Loaders holding valid handles execute concurrent imports 
safely.
+3. **Graceful Release**: When an import completes, Loader drains its workers 
and releases its handle. When the last owner releases,
+   graph mode automatically returns to `NONE`.
+4. **Failure Recovery**: If abnormal exit (e.g. `kill -9`) leaves an orphaned 
owner, release it manually via the

Review Comment:
   Important: Step 4 sends the reader to the REST release endpoint, but a 
Loader user has no token to release with, and the recovery path the paired code 
does provide is not documented on either page.
   
   A release body needs `instance`, `epoch` and `token`. In 
apache/hugegraph-toolchain#815 at 2b2ffcd, `LoadingModeOwner.toString()` 
returns only `instance/epoch`, so both the `Preparing LOADING owner {}` log 
line and the `Failed to release LOADING owner %s; stop and drain related 
imports before administrator recovery on the Server` error leave the token out, 
and the #815 Loader README says owner tokens are not logged. After `kill -9` 
nobody has the token. (Commit 4532247 also dropped the earlier sentence saying 
the handle is printed in the release error; at the current #815 head that 
sentence would be wrong anyway.)
   
   The Loader error points at the `recover` action in apache/hugegraph#3331 at 
eee07ff: `PUT .../mode/owners` with 
`{"action":"recover","instance":"...","epoch":N,"revision":N,"confirm":"I'm 
sure all imports have stopped"}`. It needs the global administrator role and 
the current `revision`, which `GET .../mode/owners` now returns next to 
`protocol`, `instance` and `epoch`. A stale request gets 409; success returns 
`state: "recovered"`, `mode: NONE` and a new epoch. Section 6.3.5 in graphs.md 
mentions neither `recover` nor `revision`.
   
   Please (1) document the `recover` action in 6.3.5, EN and CN: add `revision` 
to the GET example, plus the admin requirement, the confirm string and the 409 
on a stale request; and (2) rewrite this step and CN line 1005 as: stop and 
drain all related imports and graph writes, then use the administrator recovery 
in 6.3.5 or restart the Server. Manual `release` applies only to a caller that 
still holds its token, such as a direct REST client. The stop-and-drain part is 
what imbajin asked for in the PR conversation.



##########
content/cn/docs/quickstart/toolchain/hugegraph-loader.md:
##########
@@ -982,6 +985,32 @@ bin/hugegraph-loader.sh \
 
 本例用独立的 `docs_smoke_person` 标签和 `docs_*` 属性,避免与预置的 `person` 
样例数据冲突。成功时统计中应显示顶点写入成功数为 `2`、边写入成功数为 `0`。也可以在 Hubble 的 Gremlin 工作台查询 
`g.V().hasLabel('docs_smoke_person').values('docs_name')`,结果应包含 
`docs-smoke-alice` 和 `docs-smoke-bob`。
 
+#### 3.4.6 并发导入与 LOADING owner {#loading-ownership}
+
+自 1.8.0 版本起,Loader 支持并发导入模式所有权保护协议。
+
+```mermaid
+graph TD
+    A["图模式 NONE"] -->|首个 Loader 申请所有权| B["进入 LOADING 模式 (分配 token)"]
+    B -->|其他并发 Loader 加入| B
+    B -->|部分 Loader 导入完成释放| B
+    B -->|最后一个 Loader 释放| C["自动恢复 NONE 模式"]
+    B -.->|异常/进程中断 (Token 残留)| D["手动调 API 释放 / 重启图实例"]

Review Comment:
   Important: This diagram does not render. The edge label `|异常/进程中断 (Token 
残留)|` has unquoted parentheses, and Mermaid's flowchart parser reads `(` inside 
a pipe label as the start of a node shape:
   
   ```text
   Parse error on line 6: ...B -.->|异常/进程中断 (Token 残留)| D["手动调 A
   Expecting 'SQE', ..., 'PIPE', ..., got 'PS'
   ```
   
   I parsed it with the `mermaid.min.js` that the site theme vendors 
(pgsty/oink v1.2.0 ships Mermaid 11.17.2). The EN diagram parses, because its 
edge labels have no parentheses; parentheses inside quoted node labels are 
fine. The Hugo build cannot catch this because Mermaid runs in the browser, so 
the CN page would show an error in place of the diagram.
   
   Please quote the label, which parses:
   
   ```suggestion
       B -.->|"异常/进程中断 (Token 残留)"| D["手动调 API 释放 / 重启图实例"]
   ```



##########
content/en/docs/quickstart/toolchain/hugegraph-loader.md:
##########
@@ -986,6 +989,38 @@ bin/hugegraph-loader.sh \
 
 Expected result: two vertices and zero edges imported. This example uses 
dedicated schema names to avoid conflicts with a preloaded `person` label. In 
Hubble, run `g.V().hasLabel('docs_smoke_person').values('docs_name')` to 
confirm `docs-smoke-alice` and `docs-smoke-bob`.
 
+#### 3.4.6 Concurrent imports and LOADING ownership {#loading-ownership}
+
+Starting with HugeGraph 1.8.0, Loader supports concurrent import protection 
via the LOADING ownership protocol.
+
+```mermaid
+graph TD
+    A["Graph Mode: NONE"] -->|First Loader acquires owner| B["LOADING Mode 
(Acquires Token)"]
+    B -->|Concurrent Loaders join| B
+    B -->|Loader finishes & releases| B
+    B -->|Last Loader releases| C["Restores to NONE"]
+    B -.->|Abnormal exit / kill -9| D["Manual REST API Release / Restart"]
+    D -.-> A

Review Comment:
   Minor: The `D -.-> A` edge shows recovery always returning the graph to 
`NONE`. Releasing one orphaned handle does that only when it is the last owner: 
`LoadingMode.update` in apache/hugegraph#3331 at eee07ff sets `NONE` only when 
`active.isEmpty()`, so while other Loaders still hold handles the graph stays 
in `LOADING`. Restart and `recover` do return `NONE`. Please make the edge say 
which case it is (for example "last owner released, or recover/restart"), or 
draw the single-release case back to B. The CN diagram has the same edge at 
line 999.



##########
content/en/docs/quickstart/toolchain/hugegraph-loader.md:
##########
@@ -986,6 +989,38 @@ bin/hugegraph-loader.sh \
 
 Expected result: two vertices and zero edges imported. This example uses 
dedicated schema names to avoid conflicts with a preloaded `person` label. In 
Hubble, run `g.V().hasLabel('docs_smoke_person').values('docs_name')` to 
confirm `docs-smoke-alice` and `docs-smoke-bob`.
 
+#### 3.4.6 Concurrent imports and LOADING ownership {#loading-ownership}
+
+Starting with HugeGraph 1.8.0, Loader supports concurrent import protection 
via the LOADING ownership protocol.
+
+```mermaid
+graph TD
+    A["Graph Mode: NONE"] -->|First Loader acquires owner| B["LOADING Mode 
(Acquires Token)"]
+    B -->|Concurrent Loaders join| B
+    B -->|Loader finishes & releases| B
+    B -->|Last Loader releases| C["Restores to NONE"]
+    B -.->|Abnormal exit / kill -9| D["Manual REST API Release / Restart"]
+    D -.-> A
+```
+
+1. **Probe and Acquire**: Loader probes `/mode/owners`. The first Loader 
obtains an ownership handle (`instance/epoch/token`),
+   transitioning graph mode from `NONE` to `LOADING`; concurrent Loaders can 
join.
+2. **Data Import**: Loaders holding valid handles execute concurrent imports 
safely.
+3. **Graceful Release**: When an import completes, Loader drains its workers 
and releases its handle. When the last owner releases,
+   graph mode automatically returns to `NONE`.
+4. **Failure Recovery**: If abnormal exit (e.g. `kill -9`) leaves an orphaned 
owner, release it manually via the
+   [RESTful API release 
endpoint](/docs/clients/restful-api/graphs/#loading-ownership) or 
reopen/restart the graph.
+{.steps}
+
+> [!NOTE]
+> **Compatibility and Operational Notes**
+> - **Legacy Compatibility**: During ordinary imports, only a `404` status on 
`/mode/owners` falls back to the legacy enum `LOADING`/`NONE` flow
+>   (maintains compatibility with Server 1.7, without multi-loader 
protection). Network or permission errors do not trigger fallback.
+> - **Restore and Graph Source Imports**: Retain the `RESTORING` flow without 
ownership probes or automatic resets. After a restore import completes,
+>   manually reset graph mode to `NONE` before starting ordinary imports.
+> - **Scope & Lifecycle**: Ownership is bound to the target Server instance 
without cross-node synchronization or automatic lease expiry.
+>   For programmatic integrations, invoke `HugeGraphLoader.shutdown()` to 
ensure proper cleanup.

Review Comment:
   Minor: "invoke `HugeGraphLoader.shutdown()` to ensure proper cleanup" 
promises more than the #815 code does. At 2b2ffcd, `shutdown()` calls 
`stopThenShutdown()`, which tries the release once. If that fails, 
`unsetLoadingMode()` throws, the context is closed anyway, and the #815 README 
says "another `shutdown()` does not retry the release", so the owner stays 
active until recovery. The CN line 1012 says something else again: use 
`shutdown()` to exit safely when stopping early. Please say in both languages 
that embedded callers must call `shutdown()` so the handle is released once, 
and that a failed release leaves the owner active and needs the recovery step 
above.



##########
content/en/docs/quickstart/toolchain/hugegraph-loader.md:
##########
@@ -986,6 +989,38 @@ bin/hugegraph-loader.sh \
 
 Expected result: two vertices and zero edges imported. This example uses 
dedicated schema names to avoid conflicts with a preloaded `person` label. In 
Hubble, run `g.V().hasLabel('docs_smoke_person').values('docs_name')` to 
confirm `docs-smoke-alice` and `docs-smoke-bob`.
 
+#### 3.4.6 Concurrent imports and LOADING ownership {#loading-ownership}
+
+Starting with HugeGraph 1.8.0, Loader supports concurrent import protection 
via the LOADING ownership protocol.

Review Comment:
   Important: This says Loader supports the protocol "starting with HugeGraph 
1.8.0", but no 1.8.0 build has it, and line 10 says this guide follows 
Toolchain `master` (currently `1.8.0`). apache/hugegraph-toolchain#815 and 
apache/hugegraph#3331 are both still open; Toolchain and Server `master` 
already carry `revision` 1.8.0 and contain no `mode/owners` code, and the last 
release is 1.7.0. A reader who builds Loader from `master` as line 11 suggests 
gets the enum `LOADING`/`NONE` flow with no owner probe, while lines 30-31 tell 
them ordinary imports "acquire same-Server ownership protection". The #815 
Loader README itself says the paired Server change "must be merged and released 
before the new guarantee is available".
   
   Earlier heads of this PR carried a proposed/unreleased qualifier for this 
reason (imbajin's first inline comment); 019fb6e3 removed it. Please either put 
back one sentence in both languages, here, in the note at lines 30-31 and in 
graphs.md 6.3.5, saying the protocol needs Server #3331 and Client/Loader #815, 
which are not in `master` yet, or hold this PR until both merge.



-- 
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]

Reply via email to