bitflicker64 commented on code in PR #3134:
URL: https://github.com/apache/hugegraph/pull/3134#discussion_r4075570758
##########
hugegraph-pd/hg-pd-dist/src/assembly/static/bin/util.sh:
##########
@@ -394,3 +394,21 @@ function kill_process_and_wait() {
kill_process "$process_name" "$pid"
wait_for_shutdown "$process_name" "$pid" "$timeout_s"
}
+
+# Find HugeGraph server directory in parent path using prefix glob.
+# Usage: find_hugegraph_server_dir "/path/to/parent"
+# Returns: first matching directory path or empty string
+function find_hugegraph_server_dir() {
Review Comment:
‼️ `find_hugegraph_server_dir` has no callers. `git grep
find_hugegraph_server_dir f2f3003` returns only this definition and its copy at
`hugegraph-store/hg-store-dist/src/assembly/static/bin/util.sh:441`. Delete
both (36 lines); add it back in the PR that first calls it.
##########
.specs/hugegraph-server/ToplingDB/task.md:
##########
@@ -0,0 +1,32 @@
+# Tasks of ToplingDB Easy Migrate
+
+## Runtime Integration
+
+- [x] Keep Server, PD, and Store on the existing RocksDB Java API.
+- [x] Prepare the optional ToplingDB JAR, native library, and Web resources
+ before service startup.
+- [x] Select ToplingDB explicitly from component configuration.
+- [x] Export a readable component-specific
+ `TOPLINGDB_EASY_MIGRATE_CONF` before the JVM starts.
+- [x] Let the native Easy Migrate hook apply options and track DB/CF lifecycle.
+- [x] Keep `DBOptions.default` as the global fallback and retain the dedicated
+ `DBOptions.log` profile.
+
+## Component Integration
+
+- [x] Use `conf/toplingdb.yaml` for standalone Server.
+- [x] Use `conf/rocksdb_pd.yaml` for PD.
+- [x] Use `conf/rocksdb_store.yaml` for Store.
+- [x] Keep HStore-backed Server as a remote client without local RocksDB
+ ownership.
+
+## Lifecycle and Testing
+
+- [x] Use normal RocksDB Java close paths during graceful shutdown.
+- [x] Rely on native `MaybeForgetCF` and `MaybeForgetDB`; do not add Java-side
+ repository cleanup.
+- [x] Verify JAR/native mapping without constructing a plugin repository.
+- [x] Run every ToplingDB DB/CF functional test with Easy Migrate configuration
+ enabled.
+- [ ] Keep general Store shutdown timeout and unrelated thread-exit fixes in
Review Comment:
‼️ `.specs/` is a new top-level directory holding this PR's working notes: a
requirements list, a design doc, and a checkbox task list with an open item.
Nothing else in the repo uses `.specs/`, and the design content (architecture,
component scope, config contract, open/close lifecycle, rollback) is already in
`docs/toplingdb/toplingdb.md` and `toplingdb-development.md` from this same PR.
Delete the three files (239 lines). If a line in `design.md` is missing from
`docs/toplingdb/`, move it there.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -24,6 +24,75 @@ function command_available() {
return 1
}
+# Resolve a path before comparing it with an excluded runtime directory. The
+# fallback keeps the launcher usable on macOS hosts where `realpath` may not be
+# installed, while the depth limit prevents malformed symlink loops from
+# hanging startup.
+function canonical_path() {
+ local path="$1"
+ local depth="${2:-0}"
+ local target
+ local dir
+
+ (( depth < 40 )) || return 1
+ if command_available "realpath"; then
+ realpath "$path"
+ return
+ fi
+ if [[ -L "$path" ]]; then
+ target=$(readlink "$path") || return 1
+ if [[ "$target" = /* ]]; then
+ canonical_path "$target" "$((depth + 1))"
+ else
+ canonical_path "$(dirname "$path")/$target" "$((depth + 1))"
+ fi
+ return
+ fi
+ dir=$(cd -P "$(dirname "$path")" 2>/dev/null && pwd) || return 1
+ printf '%s/%s\n' "$dir" "$(basename "$path")"
+}
+
+# Print JAR paths below a component's lib directory except anything whose
+# canonical target lives below its optional Topling subtree. `find -P` keeps
+# directory aliases from being traversed, while direct file aliases are
+# resolved and emitted canonically after the exclusion check.
+function find_standard_lib_jars() {
Review Comment:
⚠️ This helper and `canonical_path` above it (69 lines, including a
hand-rolled `realpath` with a 40-level symlink depth limit) exist to do one
thing: keep `lib/topling/` out of the Server classpath scan. `find` already
does that with `-prune`:
```bash
CP=$(find -L "$LIB" -path "$LIB/topling" -prune -o -name
'log4j-slf4j-impl*.jar' -print | sort | tr '\n' ':')
CP="$CP":$(find -L "$LIB" -path "$LIB/topling" -prune -o -name
'hugegraph*.jar' -print | sort | tr '\n' ':')
CP="$CP":$(find -L "$LIB" -path "$LIB/topling" -prune -o -name '*.jar' \!
-name 'hugegraph*' \! -name 'log4j-slf4j-impl*.jar' -print | sort | tr '\n' ':')
```
Same in `init-store.sh`. The canonical-path check guards against a symlink
elsewhere in `lib/` pointing into `lib/topling/`; `lib/` is the operator's own
install directory, not input from outside, so that case does not need code.
Even smaller: ship the Topling JAR outside `lib/` (for example `topling/` next
to it), and the original `find` lines stay untouched.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/common-topling.sh:
##########
@@ -0,0 +1,288 @@
+#!/bin/bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+set -Eeuo pipefail
+IFS=$'\n\t'
+trap 'echo "[common-topling] error at line ${LINENO}: ${BASH_COMMAND}" >&2' ERR
+
+GITHUB="https://github.com";
+
+function abs_path() {
+ local SOURCE
+ SOURCE="${BASH_SOURCE[0]}"
+ while [[ -h "$SOURCE" ]]; do
+ local DIR
+ DIR="$(cd -P "$(dirname "$SOURCE")" && pwd)"
+ SOURCE="$(readlink "$SOURCE")"
+ [[ $SOURCE != /* ]] && SOURCE="$DIR/$SOURCE"
+ done
+ cd -P "$(dirname "$SOURCE")" && pwd
+}
+
+function extract_so_with_jar() {
Review Comment:
⚠️ `extract_so_with_jar` and `extract_html_css_from_jar` are the same 35
lines twice, and most of each is checks that cannot fail here:
`prepare_toplingdb` just found `$jar_file` with `ls`, so the existence check is
dead, and `unzip` takes a relative path, so the `realpath`/`readlink -f` branch
is not needed. What they do is:
```bash
unzip -j -o "$jar_file" '*.so' -d "$dest_dir" >/dev/null
```
The HTML/CSS half feeds the Topling HTTP monitor, which is off in all three
shipped YAMLs (`auto_start_http: false`, line 22), and the caller already
treats its failure as a warning. Drop it, or if you want it ready: `unzip -j -o
"$jar_file" '*.html' '*.css' -d "$dest_dir/rocksdb_resource" >/dev/null ||
true`. Keep one `command -v unzip` check in `prepare_toplingdb` if you want the
friendly message. The unused `abs_path` at the top of this file can go too;
`prepare-topling.sh` computes `BIN` itself.
--
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]
