Copilot commented on code in PR #2952:
URL:
https://github.com/apache/incubator-hugegraph/pull/2952#discussion_r2834023180
##########
docker/docker-compose.yml:
##########
@@ -15,44 +15,99 @@
# limitations under the License.
#
-version: "3"
+name: hugegraph-single
+
+networks:
+ hg-net:
+ driver: bridge
+
+volumes:
+ hg-pd-data:
+ hg-store-data:
services:
pd:
- image: hugegraph/pd
- container_name: pd
+ build:
+ context: ..
+ dockerfile: hugegraph-pd/Dockerfile
+ image: hugegraph/pd:${HUGEGRAPH_VERSION:-1.7.0}
+ container_name: hg-pd
hostname: pd
- network_mode: host
+ restart: unless-stopped
+ networks: [hg-net]
+ environment:
+ HG_PD_GRPC_HOST: pd
+ HG_PD_GRPC_PORT: "8686"
+ HG_PD_REST_PORT: "8620"
+ HG_PD_RAFT_ADDRESS: pd:8610
+ HG_PD_RAFT_PEERS_LIST: pd:8610
+ HG_PD_INITIAL_STORE_LIST: store:8500
+ HG_PD_DATA_PATH: /hugegraph-pd/pd_data
+ ports:
+ - "8620:8620"
+ - "8686:8686"
+ volumes:
+ - hg-pd-data:/hugegraph-pd/pd_data
healthcheck:
- test: ["CMD", "curl", "-f", "http://localhost:8620";]
+ test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/health
>/dev/null || exit 1"]
interval: 10s
timeout: 5s
- retries: 3
+ retries: 12
+ start_period: 20s
store:
- image: hugegraph/store
- container_name: store
+ build:
+ context: ..
+ dockerfile: hugegraph-store/Dockerfile
+ image: hugegraph/store:${HUGEGRAPH_VERSION:-1.7.0}
+ container_name: hg-store
hostname: store
- network_mode: host
+ restart: unless-stopped
+ networks: [hg-net]
depends_on:
pd:
condition: service_healthy
+ environment:
+ HG_STORE_PD_ADDRESS: pd:8686
+ HG_STORE_GRPC_HOST: store
+ HG_STORE_GRPC_PORT: "8500"
+ HG_STORE_REST_PORT: "8520"
+ HG_STORE_RAFT_ADDRESS: store:8510
+ HG_STORE_DATA_PATH: /hugegraph-store/storage
+ ports:
+ - "8520:8520"
+ - "8500:8500"
+ - "8510:8510"
+ volumes:
+ - hg-store-data:/hugegraph-store/storage
healthcheck:
- test: ["CMD", "curl", "-f", "http://localhost:8520";]
+ test: ["CMD-SHELL", "curl -fsS http://localhost:8520/v1/health
>/dev/null && curl -fsS http://pd:8620/v1/health >/dev/null || exit 1"]
interval: 10s
- timeout: 5s
- retries: 3
+ timeout: 10s
+ retries: 30
+ start_period: 30s
server:
- image: hugegraph/server
- container_name: server
+ build:
+ context: ..
+ dockerfile: hugegraph-server/Dockerfile-hstore
+ image: hugegraph/server:${HUGEGRAPH_VERSION:-1.7.0}
+ container_name: hg-server
hostname: server
- network_mode: host
+ restart: unless-stopped
+ networks: [hg-net]
depends_on:
store:
condition: service_healthy
+ environment:
+ HG_GRAPH: hugegraph
Review Comment:
The environment variable HG_GRAPH is set to "hugegraph" but doesn't appear
to be used by the server entrypoint script or any other configuration scripts.
If this variable is not needed, consider removing it to avoid confusion. If
it's intended for future use or user customization, consider adding a comment
explaining its purpose or implementing its usage in the entrypoint script.
```suggestion
```
##########
hugegraph-store/hg-store-dist/docker/docker-entrypoint.sh:
##########
@@ -15,8 +15,68 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+set -euo pipefail
-# start hugegraph store
-./bin/start-hugegraph-store.sh -j "$JAVA_OPTS"
+log() { echo "[hugegraph-store-entrypoint] $*"; }
+fail_on_deprecated() {
+ local old_name="$1" new_name="$2"
+ if [[ -n "${!old_name:-}" ]]; then
+ echo "ERROR: deprecated env '${old_name}' detected. Use '${new_name}'
instead." >&2
+ exit 2
+ fi
+}
+
+require_env() {
+ local name="$1"
+ if [[ -z "${!name:-}" ]]; then
+ echo "ERROR: missing required env '${name}'" >&2; exit 2
+ fi
+}
+
+json_escape() {
+ local s="$1"
+ s=${s//\\/\\\\}; s=${s//\"/\\\"}; s=${s//$'\n'/}
+ printf "%s" "$s"
+}
+
+# ── Guard deprecated vars ─────────────────────────────────────────────
+fail_on_deprecated "PD_ADDRESS" "HG_STORE_PD_ADDRESS"
+fail_on_deprecated "GRPC_HOST" "HG_STORE_GRPC_HOST"
+fail_on_deprecated "RAFT_ADDRESS" "HG_STORE_RAFT_ADDRESS"
+fail_on_deprecated "RAFT_PEERS" "HG_PD_RAFT_PEERS_LIST"
Review Comment:
The deprecated variable name suggestion is incorrect. The line suggests
using "HG_PD_RAFT_PEERS_LIST" for the deprecated "RAFT_PEERS", but this is the
Store entrypoint, not PD. Store services don't use the raft.peers-list
configuration property (only PD does). Based on the Store configuration files,
Store only has raft.address, not raft.peers-list. This deprecated variable
check should either be removed entirely, or if RAFT_PEERS was previously used
by Store, the suggested replacement variable name should be corrected to match
Store's conventions (not PD's).
```suggestion
```
##########
docker/docker-compose.yml:
##########
@@ -15,44 +15,99 @@
# limitations under the License.
#
-version: "3"
+name: hugegraph-single
+
+networks:
+ hg-net:
+ driver: bridge
+
+volumes:
+ hg-pd-data:
+ hg-store-data:
services:
pd:
- image: hugegraph/pd
- container_name: pd
+ build:
+ context: ..
+ dockerfile: hugegraph-pd/Dockerfile
+ image: hugegraph/pd:${HUGEGRAPH_VERSION:-1.7.0}
+ container_name: hg-pd
hostname: pd
- network_mode: host
+ restart: unless-stopped
+ networks: [hg-net]
+ environment:
+ HG_PD_GRPC_HOST: pd
+ HG_PD_GRPC_PORT: "8686"
+ HG_PD_REST_PORT: "8620"
+ HG_PD_RAFT_ADDRESS: pd:8610
+ HG_PD_RAFT_PEERS_LIST: pd:8610
+ HG_PD_INITIAL_STORE_LIST: store:8500
+ HG_PD_DATA_PATH: /hugegraph-pd/pd_data
+ ports:
+ - "8620:8620"
+ - "8686:8686"
+ volumes:
+ - hg-pd-data:/hugegraph-pd/pd_data
healthcheck:
- test: ["CMD", "curl", "-f", "http://localhost:8620";]
+ test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/health
>/dev/null || exit 1"]
interval: 10s
timeout: 5s
- retries: 3
+ retries: 12
+ start_period: 20s
store:
- image: hugegraph/store
- container_name: store
+ build:
+ context: ..
+ dockerfile: hugegraph-store/Dockerfile
+ image: hugegraph/store:${HUGEGRAPH_VERSION:-1.7.0}
+ container_name: hg-store
hostname: store
- network_mode: host
+ restart: unless-stopped
+ networks: [hg-net]
depends_on:
pd:
condition: service_healthy
+ environment:
+ HG_STORE_PD_ADDRESS: pd:8686
+ HG_STORE_GRPC_HOST: store
+ HG_STORE_GRPC_PORT: "8500"
+ HG_STORE_REST_PORT: "8520"
+ HG_STORE_RAFT_ADDRESS: store:8510
+ HG_STORE_DATA_PATH: /hugegraph-store/storage
+ ports:
+ - "8520:8520"
+ - "8500:8500"
+ - "8510:8510"
+ volumes:
+ - hg-store-data:/hugegraph-store/storage
healthcheck:
- test: ["CMD", "curl", "-f", "http://localhost:8520";]
+ test: ["CMD-SHELL", "curl -fsS http://localhost:8520/v1/health
>/dev/null && curl -fsS http://pd:8620/v1/health >/dev/null || exit 1"]
interval: 10s
- timeout: 5s
- retries: 3
+ timeout: 10s
+ retries: 30
+ start_period: 30s
server:
- image: hugegraph/server
- container_name: server
+ build:
+ context: ..
+ dockerfile: hugegraph-server/Dockerfile-hstore
+ image: hugegraph/server:${HUGEGRAPH_VERSION:-1.7.0}
+ container_name: hg-server
Review Comment:
The container names have been changed from "pd", "store", "server" to
"hg-pd", "hg-store", "hg-server". This is a breaking change for users who may
reference these containers by name in scripts, monitoring tools, or
documentation. While the service names (used for DNS resolution within the
Docker network) remain unchanged, external references using docker commands
will break. Consider documenting this breaking change in the PR description or
release notes, or if backward compatibility is important, keep the original
container names.
##########
hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh:
##########
@@ -15,8 +15,72 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+set -euo pipefail
-# start hugegraph pd
-./bin/start-hugegraph-pd.sh -j "$JAVA_OPTS"
+log() { echo "[hugegraph-pd-entrypoint] $*"; }
+fail_on_deprecated() {
+ local old_name="$1" new_name="$2"
+ if [[ -n "${!old_name:-}" ]]; then
+ echo "ERROR: deprecated env '${old_name}' detected. Use '${new_name}'
instead." >&2
+ exit 2
+ fi
+}
+
+require_env() {
+ local name="$1"
+ if [[ -z "${!name:-}" ]]; then
+ echo "ERROR: missing required env '${name}'" >&2; exit 2
+ fi
+}
+
+json_escape() {
+ local s="$1"
+ s=${s//\\/\\\\}; s=${s//\"/\\\"}; s=${s//$'\n'/}
+ printf "%s" "$s"
+}
+
+# ── Guard deprecated vars ─────────────────────────────────────────────
+fail_on_deprecated "GRPC_HOST" "HG_PD_GRPC_HOST"
+fail_on_deprecated "RAFT_ADDRESS" "HG_PD_RAFT_ADDRESS"
+fail_on_deprecated "RAFT_PEERS" "HG_PD_RAFT_PEERS_LIST"
+fail_on_deprecated "PD_INITIAL_STORE_LIST" "HG_PD_INITIAL_STORE_LIST"
+
+# ── Required vars ─────────────────────────────────────────────────────
+require_env "HG_PD_GRPC_HOST"
+require_env "HG_PD_RAFT_ADDRESS"
+require_env "HG_PD_RAFT_PEERS_LIST"
+require_env "HG_PD_INITIAL_STORE_LIST"
+
+# ── Defaults ──────────────────────────────────────────────────────────
+: "${HG_PD_GRPC_PORT:=8686}"
+: "${HG_PD_REST_PORT:=8620}"
+: "${HG_PD_DATA_PATH:=/hugegraph-pd/pd_data}"
+: "${HG_PD_INITIAL_STORE_COUNT:=1}"
+
+# ── Build SPRING_APPLICATION_JSON ─────────────────────────────────────
+SPRING_APPLICATION_JSON="$(cat <<JSON
+{
+ "grpc": { "host": "$(json_escape "${HG_PD_GRPC_HOST}")",
+ "port": "$(json_escape "${HG_PD_GRPC_PORT}")" },
+ "server": { "port": "$(json_escape "${HG_PD_REST_PORT}")" },
+ "raft": { "address": "$(json_escape "${HG_PD_RAFT_ADDRESS}")",
+ "peers-list": "$(json_escape "${HG_PD_RAFT_PEERS_LIST}")" },
+ "pd": { "data-path": "$(json_escape "${HG_PD_DATA_PATH}")",
+ "initial-store-list": "$(json_escape
"${HG_PD_INITIAL_STORE_LIST}")" }
Review Comment:
The HG_PD_INITIAL_STORE_COUNT environment variable has a default value of 1
set on line 59, but it's not included in the SPRING_APPLICATION_JSON
configuration (lines 62-73). This means the Spring application won't receive
this configuration value, and PD will use its default value of 3 instead. For a
single-node setup, initial-store-count should be 1 to indicate the minimum
number of store nodes required for the cluster to be considered ready. Add
"initial-store-count" to the "pd" section of SPRING_APPLICATION_JSON.
##########
docker/docker-compose.yml:
##########
@@ -15,44 +15,99 @@
# limitations under the License.
#
-version: "3"
+name: hugegraph-single
+
+networks:
+ hg-net:
+ driver: bridge
+
+volumes:
+ hg-pd-data:
+ hg-store-data:
services:
pd:
- image: hugegraph/pd
- container_name: pd
+ build:
+ context: ..
+ dockerfile: hugegraph-pd/Dockerfile
+ image: hugegraph/pd:${HUGEGRAPH_VERSION:-1.7.0}
+ container_name: hg-pd
hostname: pd
- network_mode: host
+ restart: unless-stopped
+ networks: [hg-net]
+ environment:
+ HG_PD_GRPC_HOST: pd
+ HG_PD_GRPC_PORT: "8686"
+ HG_PD_REST_PORT: "8620"
+ HG_PD_RAFT_ADDRESS: pd:8610
+ HG_PD_RAFT_PEERS_LIST: pd:8610
+ HG_PD_INITIAL_STORE_LIST: store:8500
+ HG_PD_DATA_PATH: /hugegraph-pd/pd_data
+ ports:
+ - "8620:8620"
+ - "8686:8686"
+ volumes:
+ - hg-pd-data:/hugegraph-pd/pd_data
healthcheck:
- test: ["CMD", "curl", "-f", "http://localhost:8620";]
+ test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/health
>/dev/null || exit 1"]
interval: 10s
timeout: 5s
- retries: 3
+ retries: 12
+ start_period: 20s
store:
- image: hugegraph/store
- container_name: store
+ build:
+ context: ..
+ dockerfile: hugegraph-store/Dockerfile
+ image: hugegraph/store:${HUGEGRAPH_VERSION:-1.7.0}
+ container_name: hg-store
hostname: store
- network_mode: host
+ restart: unless-stopped
+ networks: [hg-net]
depends_on:
pd:
condition: service_healthy
+ environment:
+ HG_STORE_PD_ADDRESS: pd:8686
+ HG_STORE_GRPC_HOST: store
+ HG_STORE_GRPC_PORT: "8500"
+ HG_STORE_REST_PORT: "8520"
+ HG_STORE_RAFT_ADDRESS: store:8510
+ HG_STORE_DATA_PATH: /hugegraph-store/storage
+ ports:
+ - "8520:8520"
+ - "8500:8500"
+ - "8510:8510"
+ volumes:
+ - hg-store-data:/hugegraph-store/storage
healthcheck:
- test: ["CMD", "curl", "-f", "http://localhost:8520";]
+ test: ["CMD-SHELL", "curl -fsS http://localhost:8520/v1/health
>/dev/null && curl -fsS http://pd:8620/v1/health >/dev/null || exit 1"]
Review Comment:
The healthcheck for the store service includes a check for PD's health
endpoint in addition to its own. While this ensures PD is reachable, it could
cause the store container to be marked as unhealthy even when the store service
itself is functioning correctly. If PD becomes temporarily unavailable or slow
to respond, this will incorrectly report the store as unhealthy. Consider using
only the store's own health endpoint for its healthcheck, and rely on the
depends_on with service_healthy condition for startup ordering rather than
continuous health monitoring of dependencies.
```suggestion
test: ["CMD-SHELL", "curl -fsS http://localhost:8520/v1/health
>/dev/null || exit 1"]
```
--
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]
