bitflicker64 commented on code in PR #525:
URL: https://github.com/apache/hugegraph-doc/pull/525#discussion_r4287795949
##########
content/en/docs/quickstart/toolchain/hugegraph-loader.md:
##########
@@ -986,6 +989,42 @@ 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: The note saying the protocol is not in `master` yet is gone
again. 0760b08 added it in reply to the earlier thread on this line. Then
cfb7b6b ("remove hardcoded PR references") deleted it from all six places:
EN/CN graphs.md 6.3.5, the EN/CN overview note and EN/CN 3.4.6. Nothing
replaced it.
The facts have not changed. apache/hugegraph#3331 and
apache/hugegraph-toolchain#815 are both open. Server and Toolchain `master`
both carry `<revision>1.8.0</revision>`, `git grep mode/owners origin/master`
finds nothing in either repo, and the last release is 1.7.0. Line 10 says this
guide follows Toolchain `master` (currently `1.8.0`) and tells readers to build
from source. A reader who does that against a `master` Server gets the 404
fallback in `LoadContext.setLoadingMode`, which only logs a WARN. They import
with the legacy enum flow while lines 30-31 and this line say 1.8.0 imports
"acquire same-Server ownership protection". "Earlier 1.8 builds lack this
protocol" in graphs.md does not help, because a `master` build reports itself
as 1.8.0.
Please do one of these. Hold this PR until #3331 and #815 are both merged,
so `master` matches the text. Or put back a version-neutral qualifier in both
languages, which avoids hardcoding PR numbers. For example: "Requires a Server
whose `GET .../mode/owners` returns `protocol: 1` and a matching Loader;
current `master` does not have it yet."
##########
content/en/docs/quickstart/toolchain/hugegraph-loader.md:
##########
@@ -986,6 +989,42 @@ 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 Recovery / Restart"]
+ D -.->|"Last owner released, or recover/restart"| A
+ D -.->|"Other owners still active"| B
Review Comment:
Minor: In the EN diagram, node D is now "Manual Recovery / Restart", but the
`D -.->|"Other owners still active"| B` edge only fits a single manual
`release`. Neither of D's own actions can lead back to B. `LoadingMode.recover`
in apache/hugegraph#3331 at b502b20 clears `active` and sets `NONE`. A restart
also comes back empty, because `LoadingMode` is not persisted. The edge label
on line 1003 ("Last owner released, ...") also refers to a release that D no
longer names. The CN node on line 998 still says "管理员恢复 / 释放 / 重启图实例", so the
two languages differ. Please add release back to the EN node, for example
`D["Manual Release / Recovery / Restart"]`, so it matches CN and both outgoing
edges.
##########
dist/validate-release.sh:
##########
@@ -709,7 +709,7 @@ stop_services() {
client_classpath() {
# Match ordinary Loader: its fat JAR relocates okhttp3 and must not mix
with the regular SDK.
- find -L "$1/lib" -name '*.jar' ! -name
'apache-hugegraph-loader-*-shaded.jar' \
+ find -L "$1/lib" -maxdepth 1 -type f -name '*.jar' ! -name
'apache-hugegraph-loader-*-shaded.jar' \
Review Comment:
Minor: This change and the `auth.md` edit (which drops the
apache/hugegraph#3096 link) have nothing to do with Loader mode ownership. The
PR description does not mention either of them. Also, the comment above still
says "Match ordinary Loader", but
`hugegraph-loader/assembly/static/bin/hugegraph-loader.sh` on Toolchain
`master` builds its classpath with `find -L "$LIB" -name '*.jar'` and no
`-maxdepth`. So the two now differ. The Loader `lib/` is flat today, so nothing
breaks yet. Please move both changes to their own PR, or update the comment and
mention them in the PR description, so the release-validation change gets its
own review.
--
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]
