From 1b4702993e9ebbd797496a51f9a4f28198ad577a Mon Sep 17 00:00:00 2001 From: imbajin Date: Mon, 12 Oct 2026 20:41:27 +0800 Subject: [PATCH 01/11] fix(docker): expose Server discovery identity - map service identity and graphspace environment inputs - expose Helm settings with existing default behavior - validate effective identity inputs before writing config - document naming constraints and paired website guidance --- .github/workflows/helm-chart-ci.yml | 11 ++++ docker/README.md | 29 ++++++++++ helm/hugegraph/README.md | 29 ++++++++++ helm/hugegraph/templates/_helpers.tpl | 2 +- .../templates/server-deployment.yaml | 8 +++ .../tests/server_discovery_test.yaml | 46 +++++++++++++++ helm/hugegraph/values.schema.json | 12 ++++ helm/hugegraph/values.yaml | 5 ++ .../docker/docker-entrypoint-test.sh | 56 +++++++++++++++++++ .../docker/docker-entrypoint.sh | 20 +++++++ 10 files changed, 217 insertions(+), 1 deletion(-) diff --git a/.github/workflows/helm-chart-ci.yml b/.github/workflows/helm-chart-ci.yml index 6e425a3dca..a01180729c 100644 --- a/.github/workflows/helm-chart-ci.yml +++ b/.github/workflows/helm-chart-ci.yml @@ -151,6 +151,17 @@ jobs: helm template ci helm/hugegraph \ --set "${component}.probes.readiness.successThreshold=2" > /dev/null done + # These deployment inputs must serialize unchanged through the properties reader. + must_fail --set-string server.serviceId=café + must_fail --set-string server.serviceId=😀 + must_fail --set-string "$(printf 'server.serviceId=ops\tteam')" + # Match the Server API graph-space names, including its 48-character limit. + must_fail --set-string server.graphSpace=TeamA + must_fail --set-string server.graphSpace=team-a + must_fail --set-string server.graphSpace=1team + graph_space_max=$(printf 'a%.0s' {1..48}) + helm template ci helm/hugegraph --set-string "server.graphSpace=${graph_space_max}" > /dev/null + must_fail --set-string "server.graphSpace=${graph_space_max}a" must_fail --set pd.replicas=100 must_fail --set pd.pdb.minAvailable=3 must_fail --set server.hpa.enabled=true diff --git a/docker/README.md b/docker/README.md index 869b5d819e..a79737d7f3 100644 --- a/docker/README.md +++ b/docker/README.md @@ -269,6 +269,35 @@ Standalone stores RocksDB data at `/hugegraph-server/rocksdb-data`. The HStore t `docker compose down` keeps named-volume data. `docker compose down -v` intentionally deletes it. +### Server service identity + +Images built from current master accept `HG_SERVER_SERVICE_ID` and +`HG_SERVER_GRAPHSPACE`, which write `server.service_id` and `server.graphspace` +in `conf/rest-server.properties`. A property keeps its existing value when both +its `HG_SERVER_*` variable and its older name are unset or empty; the shipped +defaults are both `DEFAULT`. +The older `SERVICE_ID` and `GRAPH_SPACE` variables emitted by the Kubernetes driver +are also accepted. A non-empty `HG_SERVER_*` value takes precedence over its older name. +These container identity inputs accept printable ASCII (bytes 0x20–0x7E); +spaces and backslashes in service IDs round-trip unchanged. Non-ASCII and control +characters are rejected before the entrypoint writes configuration, because the +Server reads properties using ISO-8859-1. This is the container input's +serialization limit; it does not change the core service-name contract. + +For a PD-backed Server, add the variables to the Server container's environment, +for example `HG_SERVER_SERVICE_ID=analytics` and `HG_SERVER_GRAPHSPACE=team_a`. +Create `team_a` in the existing cluster before starting the assigned Server; +these settings do not create graph spaces. The Compose files do not pass these +variables through from the host, so add explicit `environment:` entries to a +local Compose override. The Helm chart exposes the same settings as +[`server.serviceId` and `server.graphSpace`](../helm/hugegraph/README.md#server-service-identity). + +This selects the Server's PD service metadata. It does not change +`server.path_graphspace` for REST paths that omit `/graphspaces/`, or +configure Hubble's chosen service. Actual non-default service discovery, +authentication and graph-space isolation still require verification with a +running Linux cluster. + ### Server logs and crash files The Server container owns its foreground launcher and returns the JVM's exit status after startup, diff --git a/helm/hugegraph/README.md b/helm/hugegraph/README.md index ae74b35a43..8b97c9827c 100644 --- a/helm/hugegraph/README.md +++ b/helm/hugegraph/README.md @@ -567,6 +567,8 @@ default values. | `server.port` | Server REST port, container port, and Service port | `8080` | | `server.readinessPath` | Path the Server readinessProbe hits. Set `/readiness` once the Server image serves it (apache/hugegraph#3212); it answers 503 while the Server cannot serve graph traffic. Startup and liveness stay on `/versions` | `/versions` | | `server.backend` | Storage backend | `hstore` | +| `server.serviceId` | Non-default PD service name; `DEFAULT` preserves the image config | `DEFAULT` | +| `server.graphSpace` | Existing non-default graph space for the PD service; `DEFAULT` preserves the image config | `DEFAULT` | | `server.resources` | Server resources. `requests.cpu` is required when HPA is enabled | `{}` | | `server.podSecurityContext` | Pod-level securityContext, rendered only when set | `{}` | | `server.securityContext` | Container-level securityContext. Hardened by default; `runAsNonRoot` is not set because the published images run as root | `allowPrivilegeEscalation: false`, `capabilities.drop: [ALL]`, `seccompProfile: RuntimeDefault` | @@ -622,6 +624,33 @@ utilization-based HPA requires a strictly positive `server.resources.requests.cpu`. +### Server service identity + +`server.serviceId` and `server.graphSpace` select the service metadata that +Servers use with PD. Both default to `DEFAULT`, which leaves the image's +`server.service_id` and `server.graphspace` untouched. A non-default value +injects `HG_SERVER_SERVICE_ID` or `HG_SERVER_GRAPHSPACE` for the image entrypoint. +`server.serviceId` accepts printable ASCII (bytes 0x20–0x7E), including spaces +and backslashes. The chart and entrypoint reject non-ASCII and control characters +to prevent the Server's ISO-8859-1 properties reader from changing the value; +this is a deployment input serialization limit, not a change to core service names: + +```bash +helm upgrade --install hg ./helm/hugegraph \ + --set-string server.serviceId=analytics \ + --set-string server.graphSpace=team_a +``` + +Use images built with these entrypoint mappings. Create the non-default graph +space in the existing cluster before deploying Servers assigned to it; the chart +does not create it. A graph-space name is `DEFAULT` or up to 48 lowercase letters, +digits and underscores, starting with a letter, as required by the Server API. All replicas in this Server Deployment use the same identity. +Changing the graph space changes the Server's assigned space; it does not move +existing graphs or change `server.path_graphspace`. Use explicit +`/graphspaces/team_a/...` REST paths, and select the matching space and service in +Hubble. Rendering and entrypoint checks verify configuration only; non-default +PD discovery, authentication and isolation require a running Linux cluster. + ### Reaching Hubble (pick one path) Most people should stop at **1**. Use **2** only if Hubble must run outside diff --git a/helm/hugegraph/templates/_helpers.tpl b/helm/hugegraph/templates/_helpers.tpl index edf747252d..d9dbcedd25 100644 --- a/helm/hugegraph/templates/_helpers.tpl +++ b/helm/hugegraph/templates/_helpers.tpl @@ -851,7 +851,7 @@ start-hugegraph-pd.sh, start-hugegraph-store.sh, and hugegraph-server.sh). {{- $reservedEnv := dict "pd" (list "HG_PD_GRPC_HOST" "HG_PD_GRPC_PORT" "HG_PD_REST_PORT" "HG_PD_RAFT_ADDRESS" "HG_PD_RAFT_PEERS_LIST" "HG_PD_INITIAL_STORE_LIST" "HG_PD_INITIAL_STORE_COUNT" "HG_PD_DATA_PATH" "HG_PD_AUTH_SECRET_KEY" "JAVA_OPTS" "JAVA_OPTIONS") "store" (list "HG_STORE_PD_ADDRESS" "HG_STORE_GRPC_HOST" "HG_STORE_GRPC_PORT" "HG_STORE_REST_PORT" "HG_STORE_RAFT_ADDRESS" "HG_STORE_DATA_PATH" "JAVA_OPTS" "JAVA_OPTIONS") - "server" (list "POD_IP" "HG_SERVER_BACKEND" "HG_SERVER_PD_PEERS" "HG_SERVER_PD_REST_ENDPOINT" "STORE_REST" "HG_SERVER_INIT_STORE_ENABLED" "HG_SERVER_URLS_TO_PD" "HG_SERVER_STARTUP_TIMEOUT_S" "PD_AUTH_PASSWORD" "PASSWORD" "HG_SERVER_AUTH_TOKEN_SECRET" "JAVA_OPTS" "JAVA_OPTIONS") + "server" (list "POD_IP" "HG_SERVER_SERVICE_ID" "HG_SERVER_GRAPHSPACE" "HG_SERVER_BACKEND" "HG_SERVER_PD_PEERS" "HG_SERVER_PD_REST_ENDPOINT" "STORE_REST" "HG_SERVER_INIT_STORE_ENABLED" "HG_SERVER_URLS_TO_PD" "HG_SERVER_STARTUP_TIMEOUT_S" "PD_AUTH_PASSWORD" "PASSWORD" "HG_SERVER_AUTH_TOKEN_SECRET" "JAVA_OPTS" "JAVA_OPTIONS") "hubble" (list "HG_HUBBLE_PD_PEERS" "HG_HUBBLE_PD_SERVER" "HG_HUBBLE_PD_PASSWORD" "HG_HUBBLE_STORE_TARGETS" "HG_HUBBLE_SERVER_URL" "SPRING_DATASOURCE_URL") -}} {{- range $component, $reserved := $reservedEnv -}} {{- $componentValues := get $.Values $component | default dict -}} diff --git a/helm/hugegraph/templates/server-deployment.yaml b/helm/hugegraph/templates/server-deployment.yaml index 48d24ebaf9..f5b75dca4b 100644 --- a/helm/hugegraph/templates/server-deployment.yaml +++ b/helm/hugegraph/templates/server-deployment.yaml @@ -258,6 +258,14 @@ spec: fieldPath: status.podIP - name: HG_SERVER_BACKEND value: {{ .Values.server.backend | quote }} + {{- if ne (.Values.server.serviceId | default "DEFAULT") "DEFAULT" }} + - name: HG_SERVER_SERVICE_ID + value: {{ .Values.server.serviceId | quote }} + {{- end }} + {{- if ne (.Values.server.graphSpace | default "DEFAULT") "DEFAULT" }} + - name: HG_SERVER_GRAPHSPACE + value: {{ .Values.server.graphSpace | quote }} + {{- end }} - name: HG_SERVER_PD_PEERS value: {{ include "hugegraph.pd.grpcPeersList" . | quote }} - name: HG_SERVER_PD_REST_ENDPOINT diff --git a/helm/hugegraph/tests/server_discovery_test.yaml b/helm/hugegraph/tests/server_discovery_test.yaml index 63aca4db3d..e06bc416b4 100644 --- a/helm/hugegraph/tests/server_discovery_test.yaml +++ b/helm/hugegraph/tests/server_discovery_test.yaml @@ -47,3 +47,49 @@ tests: asserts: - failedTemplate: errorPattern: "POD_IP" + + - it: preserves the image service identity by default + asserts: + - notExists: + path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_SERVICE_ID")] + - notExists: + path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_GRAPHSPACE")] + + - it: selects a non-default service identity + set: + server.serviceId: analytics + server.graphSpace: team_a + asserts: + - equal: + path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_SERVICE_ID")].value + value: analytics + - equal: + path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_GRAPHSPACE")].value + value: team_a + + - it: refuses an extraEnv that shadows the service ID + set: + server.extraEnv: + - name: HG_SERVER_SERVICE_ID + value: shadow + asserts: + - failedTemplate: + errorPattern: "HG_SERVER_SERVICE_ID" + + - it: refuses an extraEnv that shadows the graph space + set: + server.extraEnv: + - name: HG_SERVER_GRAPHSPACE + value: shadow + asserts: + - failedTemplate: + errorPattern: "HG_SERVER_GRAPHSPACE" + + - it: retains a service name containing spaces + set: + server.serviceId: ops team + server.graphSpace: team_a + asserts: + - equal: + path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_SERVICE_ID")].value + value: ops team diff --git a/helm/hugegraph/values.schema.json b/helm/hugegraph/values.schema.json index afce158aa8..6770e10db2 100644 --- a/helm/hugegraph/values.schema.json +++ b/helm/hugegraph/values.schema.json @@ -762,6 +762,18 @@ "probes" ], "properties": { + "serviceId": { + "type": "string", + "minLength": 1, + "pattern": "^[ -~]+$", + "description": "Printable ASCII PD service name (bytes 0x20-0x7E), including spaces and backslashes. DEFAULT preserves the image config." + }, + "graphSpace": { + "type": "string", + "minLength": 1, + "pattern": "^(DEFAULT|[a-z][a-z0-9_]{0,47})$", + "description": "Existing PD graph space: DEFAULT or up to 48 lowercase letters, digits and underscores, starting with a letter. DEFAULT preserves the image config." + }, "replicas": { "type": "integer", "minimum": 1 diff --git a/helm/hugegraph/values.yaml b/helm/hugegraph/values.yaml index 6205e17396..b99ef12d8d 100644 --- a/helm/hugegraph/values.yaml +++ b/helm/hugegraph/values.yaml @@ -274,6 +274,11 @@ store: server: replicas: 3 + # DEFAULT preserves the image config; non-default values select PD service metadata. + # serviceId accepts printable ASCII, including spaces and backslashes. + # Create a non-default graph space before starting a Server assigned to it. + serviceId: DEFAULT + graphSpace: DEFAULT image: repository: hugegraph/server # TODO: leave tag empty to inherit Chart.appVersion after the 1.8.0 diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh index 1cfa7915a6..294935ed30 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh @@ -387,6 +387,62 @@ done (cd "${TEST_HOME}"; LC_ALL=en_US.UTF-8 PASSWORD='locale probe' \ HG_SERVER_AUTH_TOKEN_SECRET="${fake_key}" bash ./docker-entrypoint.sh) [[ "$(cat "${TEST_HOME}/docker/init-store-locale")" == 'en_US.UTF-8' ]] +# Identity mappings apply on every start and leave mounted config alone when unset. +identity_conf="${TEST_HOME}/conf/rest-server.properties" +printf '\nserver.service_id=mounted-service\nserver.graphspace=mounted_space\n' >> "${identity_conf}" +(cd "${TEST_HOME}"; bash ./docker-entrypoint.sh) +grep -qx 'server.service_id=mounted-service' "${identity_conf}" +grep -qx 'server.graphspace=mounted_space' "${identity_conf}" +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID=DEFAULT HG_SERVER_GRAPHSPACE=DEFAULT \ + bash ./docker-entrypoint.sh) +grep -qx 'server.service_id=DEFAULT' "${identity_conf}" +grep -qx 'server.graphspace=DEFAULT' "${identity_conf}" +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID=service-a HG_SERVER_GRAPHSPACE=space_a bash ./docker-entrypoint.sh) +grep -qx 'server.service_id=service-a' "${identity_conf}" +grep -qx 'server.graphspace=space_a' "${identity_conf}" +(cd "${TEST_HOME}"; SERVICE_ID=legacy-service GRAPH_SPACE=legacy_space bash ./docker-entrypoint.sh) +grep -qx 'server.service_id=legacy-service' "${identity_conf}" +grep -qx 'server.graphspace=legacy_space' "${identity_conf}" +(cd "${TEST_HOME}"; SERVICE_ID=legacy-service GRAPH_SPACE=legacy_space \ + HG_SERVER_SERVICE_ID=service-b HG_SERVER_GRAPHSPACE=space_b bash ./docker-entrypoint.sh) +grep -qx 'server.service_id=service-b' "${identity_conf}" +grep -qx 'server.graphspace=space_b' "${identity_conf}" +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID= HG_SERVER_GRAPHSPACE= bash ./docker-entrypoint.sh) +grep -qx 'server.service_id=service-b' "${identity_conf}" +grep -qx 'server.graphspace=space_b' "${identity_conf}" + +# Empty new variables still allow the legacy driver values to apply. +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID= HG_SERVER_GRAPHSPACE= \ + SERVICE_ID=legacy-empty GRAPH_SPACE=legacy_empty bash ./docker-entrypoint.sh) +grep -qx 'server.service_id=legacy-empty' "${identity_conf}" +grep -qx 'server.graphspace=legacy_empty' "${identity_conf}" + +# Service permits spaces; Unicode escapes round-trip through the Server parser. +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID='ops team' bash ./docker-entrypoint.sh) +grep -Fxq 'server.service_id=ops\u0020team' "${identity_conf}" +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID=' ops team ' bash ./docker-entrypoint.sh) +grep -Fxq 'server.service_id=\u0020ops\u0020team\u0020' "${identity_conf}" +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID='ops\ team' bash ./docker-entrypoint.sh) +grep -Fxq 'server.service_id=ops\\\u0020team' "${identity_conf}" + +# Unsupported effective identity input must fail before changing mounted config. +cp "${identity_conf}" "${identity_conf}.before-invalid-identity" +for identity_name in HG_SERVER_SERVICE_ID HG_SERVER_GRAPHSPACE SERVICE_ID GRAPH_SPACE; do + for identity_value in 'café' '😀' $'ops\tteam' $'ops\nteam'; do + if (cd "${TEST_HOME}"; env "${identity_name}=${identity_value}" \ + bash ./docker-entrypoint.sh > ./docker/invalid-identity.log 2>&1); then + echo "unsupported ${identity_name} unexpectedly succeeded" >&2 + exit 1 + fi + grep -q 'must be printable ASCII' "${TEST_HOME}/docker/invalid-identity.log" + cmp "${identity_conf}.before-invalid-identity" "${identity_conf}" + done +done +# Ignored legacy input does not override a supported non-empty new value. +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID=analytics SERVICE_ID='café' \ + HG_SERVER_GRAPHSPACE=team_a GRAPH_SPACE='😀' bash ./docker-entrypoint.sh) +grep -qx 'server.service_id=analytics' "${identity_conf}" +grep -qx 'server.graphspace=team_a' "${identity_conf}" echo "PASS: Docker entrypoint configures HStore discovery and authentication" bash "${SCRIPT_DIR}/test/test-docker-entrypoint-process.sh" diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index 5b804e5ba8..750e7cec2b 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -109,6 +109,18 @@ migrate_env() { migrate_env "BACKEND" "HG_SERVER_BACKEND" migrate_env "PD_PEERS" "HG_SERVER_PD_PEERS" +migrate_env "SERVICE_ID" "HG_SERVER_SERVICE_ID" +migrate_env "GRAPH_SPACE" "HG_SERVER_GRAPHSPACE" + +# The Server reads these properties as ISO-8859-1. Reject unsupported input +# before writing UTF-8 bytes that would select a different service identity. +for identity_name in HG_SERVER_SERVICE_ID HG_SERVER_GRAPHSPACE; do + if [[ -n "${!identity_name:-}" ]] && + (( $(printf '%s' "${!identity_name}" | LC_ALL=C tr -d '\040-\176' | wc -c) > 0 )); then + log "ERROR: ${identity_name} must be printable ASCII (bytes 0x20-0x7E)" + exit 1 + fi +done if [[ -n "${HG_SERVER_AUTH_TOKEN_SECRET:-}" ]]; then if (( $(printf '%s' "${HG_SERVER_AUTH_TOKEN_SECRET}" | LC_ALL=C wc -c) < 32 )); then @@ -187,6 +199,14 @@ fi set_prop "pd.peers" "${HG_SERVER_PD_PEERS}" "${REST_SERVER_CONF}" [[ -n "${HG_SERVER_CLUSTER:-}" ]] && \ set_prop "cluster" "${HG_SERVER_CLUSTER}" "${REST_SERVER_CONF}" +if [[ -n "${HG_SERVER_SERVICE_ID:-}" ]]; then + # Service names can contain spaces. Commons Configuration keeps a backslash + # before a literal space, but decodes Unicode escapes, including at the edges. + service_id_encoded=$(encode_prop_value "${HG_SERVER_SERVICE_ID}" | sed 's/\\ /\\u0020/g') + set_prop_encoded "server.service_id" "${service_id_encoded}" "${REST_SERVER_CONF}" +fi +[[ -n "${HG_SERVER_GRAPHSPACE:-}" ]] && \ + set_prop "server.graphspace" "${HG_SERVER_GRAPHSPACE}" "${REST_SERVER_CONF}" [[ -n "${HG_SERVER_REST_URL:-}" ]] && set_prop "restserver.url" \ "${HG_SERVER_REST_URL}" "${REST_SERVER_CONF}" [[ -n "${HG_SERVER_MIN_FREE_MEMORY:-}" ]] && set_prop "restserver.min_free_memory" \ From 10a662d3503ef879ae312af74382e72e0bf8e63a Mon Sep 17 00:00:00 2001 From: imbajin Date: Mon, 12 Oct 2026 21:12:48 +0800 Subject: [PATCH 02/11] fix(docker): validate graphspace inputs - check effective graphspace names before writing config - stabilize name validation with a scoped C locale - cover new and legacy names plus locale boundaries --- docker/README.md | 4 ++ .../docker/docker-entrypoint-test.sh | 40 +++++++++++++++++++ .../docker/docker-entrypoint.sh | 10 +++++ 3 files changed, 54 insertions(+) diff --git a/docker/README.md b/docker/README.md index a79737d7f3..7c08def43a 100644 --- a/docker/README.md +++ b/docker/README.md @@ -283,6 +283,10 @@ spaces and backslashes in service IDs round-trip unchanged. Non-ASCII and contro characters are rejected before the entrypoint writes configuration, because the Server reads properties using ISO-8859-1. This is the container input's serialization limit; it does not change the core service-name contract. +`HG_SERVER_GRAPHSPACE` and a migrated `GRAPH_SPACE` must also match the Server +API graph-space rule: `DEFAULT`, or up to 48 lowercase letters, digits and +underscores, starting with a letter. Invalid environment values are rejected +before writing configuration; existing mounted properties are preserved. For a PD-backed Server, add the variables to the Server container's environment, for example `HG_SERVER_SERVICE_ID=analytics` and `HG_SERVER_GRAPHSPACE=team_a`. diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh index 294935ed30..0b0f97d4bb 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh @@ -444,5 +444,45 @@ done grep -qx 'server.service_id=analytics' "${identity_conf}" grep -qx 'server.graphspace=team_a' "${identity_conf}" +# Apply the existing graph-space naming rule to effective new and legacy input. +graph_space_max=$(printf 'a%.0s' {1..48}) +cp "${identity_conf}" "${identity_conf}.before-invalid-graphspace" +for graphspace_env in HG_SERVER_GRAPHSPACE GRAPH_SPACE; do + for graphspace in TeamA 1team team-a "${graph_space_max}a"; do + if (cd "${TEST_HOME}"; env "${graphspace_env}=${graphspace}" \ + bash ./docker-entrypoint.sh > ./docker/invalid-graphspace.log 2>&1); then + echo "invalid ${graphspace_env} unexpectedly succeeded" >&2 + exit 1 + fi + grep -q 'HG_SERVER_GRAPHSPACE must be DEFAULT' "${TEST_HOME}/docker/invalid-graphspace.log" + cmp "${identity_conf}.before-invalid-graphspace" "${identity_conf}" + done + for graphspace in DEFAULT team_a "${graph_space_max}"; do + (cd "${TEST_HOME}"; env "${graphspace_env}=${graphspace}" bash ./docker-entrypoint.sh) + grep -Fxq "server.graphspace=${graphspace}" "${identity_conf}" + done + cp "${identity_conf}" "${identity_conf}.before-invalid-graphspace" +done +(cd "${TEST_HOME}"; HG_SERVER_GRAPHSPACE=team_a GRAPH_SPACE=TeamA bash ./docker-entrypoint.sh) +grep -qx 'server.graphspace=team_a' "${identity_conf}" + +# Some installed locales make [a-z] include uppercase letters; validation owns its locale. +for identity_locale in sk_SK.ISO8859-2 cs_CZ.ISO8859-2; do + case "$(LC_ALL="${identity_locale}" locale charmap 2>/dev/null)" in + ISO8859-2 | ISO-8859-2) ;; + *) continue ;; + esac + cp "${identity_conf}" "${identity_conf}.before-invalid-locale" + for graphspace_env in HG_SERVER_GRAPHSPACE GRAPH_SPACE; do + if (cd "${TEST_HOME}"; env LC_ALL="${identity_locale}" "${graphspace_env}=TeamA" \ + bash ./docker-entrypoint.sh > ./docker/invalid-graphspace-locale.log 2>&1); then + echo "invalid ${graphspace_env} under ${identity_locale} unexpectedly succeeded" >&2 + exit 1 + fi + grep -q 'HG_SERVER_GRAPHSPACE must be DEFAULT' "${TEST_HOME}/docker/invalid-graphspace-locale.log" + cmp "${identity_conf}.before-invalid-locale" "${identity_conf}" + done +done + echo "PASS: Docker entrypoint configures HStore discovery and authentication" bash "${SCRIPT_DIR}/test/test-docker-entrypoint-process.sh" diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index 750e7cec2b..1f32dac7d5 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -122,6 +122,16 @@ for identity_name in HG_SERVER_SERVICE_ID HG_SERVER_GRAPHSPACE; do fi done +# Match the Server API graph-space naming contract after legacy migration. +if [[ -n "${HG_SERVER_GRAPHSPACE:-}" ]] && ! ( + export LC_ALL=C + [[ "${HG_SERVER_GRAPHSPACE}" =~ ^(DEFAULT|[a-z][a-z0-9_]{0,47})$ ]] +); then + log "ERROR: HG_SERVER_GRAPHSPACE must be DEFAULT or a name starting with" \ + "a lowercase letter, followed by lowercase letters, digits or underscores (1-48 characters)" + exit 1 +fi + if [[ -n "${HG_SERVER_AUTH_TOKEN_SECRET:-}" ]]; then if (( $(printf '%s' "${HG_SERVER_AUTH_TOKEN_SECRET}" | LC_ALL=C wc -c) < 32 )); then log "ERROR: HG_SERVER_AUTH_TOKEN_SECRET must be at least 32 bytes" From 21e4ffabcee22bd0127a3f51703835f945952d95 Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 02:20:42 +0800 Subject: [PATCH 03/11] fix(server): preserve literal Helm service IDs - reserve legacy identity environment variables - escape dollars before Kubernetes environment expansion - verify current HugeConfig readback in Docker CI --- helm/hugegraph/README.md | 7 +++-- helm/hugegraph/templates/_helpers.tpl | 2 +- .../templates/server-deployment.yaml | 2 +- .../tests/server_discovery_test.yaml | 26 ++++++++++++++++ .../docker/docker-entrypoint-test.sh | 25 +++++++++++++-- .../docker/test/ConfigReadback.java | 31 +++++++++++++++++++ 6 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 hugegraph-server/hugegraph-dist/docker/test/ConfigReadback.java diff --git a/helm/hugegraph/README.md b/helm/hugegraph/README.md index 8b97c9827c..558ba906ef 100644 --- a/helm/hugegraph/README.md +++ b/helm/hugegraph/README.md @@ -631,7 +631,9 @@ Servers use with PD. Both default to `DEFAULT`, which leaves the image's `server.service_id` and `server.graphspace` untouched. A non-default value injects `HG_SERVER_SERVICE_ID` or `HG_SERVER_GRAPHSPACE` for the image entrypoint. `server.serviceId` accepts printable ASCII (bytes 0x20–0x7E), including spaces -and backslashes. The chart and entrypoint reject non-ASCII and control characters +and backslashes. Dollar signs are escaped in the manifest so Kubernetes passes +the service ID literally, including `$(POD_IP)` and `$$`. The chart and entrypoint +reject non-ASCII and control characters to prevent the Server's ISO-8859-1 properties reader from changing the value; this is a deployment input serialization limit, not a change to core service names: @@ -794,7 +796,8 @@ before anything reaches the cluster: the whole membership); disable the PD PDB or use an odd replica count. - `extraEnv` must not set chart-managed variable names (for example `HG_SERVER_INIT_STORE_ENABLED` or the PD/Store identity and topology - variables): entries render after the chart-owned variables and the last + variables, including the legacy Server names `SERVICE_ID` and `GRAPH_SPACE`): + entries render after the chart-owned variables and the last duplicate wins, so an override would silently bypass a validated contract. `JAVA_OPTS` and `JAVA_OPTIONS` are reserved for the same reason: the component start scripts skip automatic heap sizing and drop the chart's diff --git a/helm/hugegraph/templates/_helpers.tpl b/helm/hugegraph/templates/_helpers.tpl index d9dbcedd25..e21b85aec0 100644 --- a/helm/hugegraph/templates/_helpers.tpl +++ b/helm/hugegraph/templates/_helpers.tpl @@ -851,7 +851,7 @@ start-hugegraph-pd.sh, start-hugegraph-store.sh, and hugegraph-server.sh). {{- $reservedEnv := dict "pd" (list "HG_PD_GRPC_HOST" "HG_PD_GRPC_PORT" "HG_PD_REST_PORT" "HG_PD_RAFT_ADDRESS" "HG_PD_RAFT_PEERS_LIST" "HG_PD_INITIAL_STORE_LIST" "HG_PD_INITIAL_STORE_COUNT" "HG_PD_DATA_PATH" "HG_PD_AUTH_SECRET_KEY" "JAVA_OPTS" "JAVA_OPTIONS") "store" (list "HG_STORE_PD_ADDRESS" "HG_STORE_GRPC_HOST" "HG_STORE_GRPC_PORT" "HG_STORE_REST_PORT" "HG_STORE_RAFT_ADDRESS" "HG_STORE_DATA_PATH" "JAVA_OPTS" "JAVA_OPTIONS") - "server" (list "POD_IP" "HG_SERVER_SERVICE_ID" "HG_SERVER_GRAPHSPACE" "HG_SERVER_BACKEND" "HG_SERVER_PD_PEERS" "HG_SERVER_PD_REST_ENDPOINT" "STORE_REST" "HG_SERVER_INIT_STORE_ENABLED" "HG_SERVER_URLS_TO_PD" "HG_SERVER_STARTUP_TIMEOUT_S" "PD_AUTH_PASSWORD" "PASSWORD" "HG_SERVER_AUTH_TOKEN_SECRET" "JAVA_OPTS" "JAVA_OPTIONS") + "server" (list "POD_IP" "HG_SERVER_SERVICE_ID" "HG_SERVER_GRAPHSPACE" "SERVICE_ID" "GRAPH_SPACE" "HG_SERVER_BACKEND" "HG_SERVER_PD_PEERS" "HG_SERVER_PD_REST_ENDPOINT" "STORE_REST" "HG_SERVER_INIT_STORE_ENABLED" "HG_SERVER_URLS_TO_PD" "HG_SERVER_STARTUP_TIMEOUT_S" "PD_AUTH_PASSWORD" "PASSWORD" "HG_SERVER_AUTH_TOKEN_SECRET" "JAVA_OPTS" "JAVA_OPTIONS") "hubble" (list "HG_HUBBLE_PD_PEERS" "HG_HUBBLE_PD_SERVER" "HG_HUBBLE_PD_PASSWORD" "HG_HUBBLE_STORE_TARGETS" "HG_HUBBLE_SERVER_URL" "SPRING_DATASOURCE_URL") -}} {{- range $component, $reserved := $reservedEnv -}} {{- $componentValues := get $.Values $component | default dict -}} diff --git a/helm/hugegraph/templates/server-deployment.yaml b/helm/hugegraph/templates/server-deployment.yaml index f5b75dca4b..db66db59d4 100644 --- a/helm/hugegraph/templates/server-deployment.yaml +++ b/helm/hugegraph/templates/server-deployment.yaml @@ -260,7 +260,7 @@ spec: value: {{ .Values.server.backend | quote }} {{- if ne (.Values.server.serviceId | default "DEFAULT") "DEFAULT" }} - name: HG_SERVER_SERVICE_ID - value: {{ .Values.server.serviceId | quote }} + value: {{ .Values.server.serviceId | replace "$" "$$" | quote }} {{- end }} {{- if ne (.Values.server.graphSpace | default "DEFAULT") "DEFAULT" }} - name: HG_SERVER_GRAPHSPACE diff --git a/helm/hugegraph/tests/server_discovery_test.yaml b/helm/hugegraph/tests/server_discovery_test.yaml index e06bc416b4..92d5adfaaa 100644 --- a/helm/hugegraph/tests/server_discovery_test.yaml +++ b/helm/hugegraph/tests/server_discovery_test.yaml @@ -93,3 +93,29 @@ tests: - equal: path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_SERVICE_ID")].value value: ops team + + - it: refuses an extraEnv that shadows the legacy service ID + set: + server.extraEnv: + - name: SERVICE_ID + value: shadow + asserts: + - failedTemplate: + errorPattern: "SERVICE_ID" + + - it: refuses an extraEnv that shadows the legacy graph space + set: + server.extraEnv: + - name: GRAPH_SPACE + value: shadow + asserts: + - failedTemplate: + errorPattern: "GRAPH_SPACE" + + - it: escapes all dollars so Kubernetes retains a literal service ID + set: + server.serviceId: 'x-$(POD_IP)-$$(POD_IP)-$(UNKNOWN)-$name-$' + asserts: + - equal: + path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_SERVICE_ID")].value + value: 'x-$$(POD_IP)-$$$$(POD_IP)-$$(UNKNOWN)-$$name-$$' diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh index 0b0f97d4bb..bb88d06739 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh @@ -72,7 +72,8 @@ chmod +x "${TEST_HOME}/bin/"*.sh if [[ -n "${HG_CONFIG_TEST_CLASSPATH:-}" ]]; then javac -cp "${HG_CONFIG_TEST_CLASSPATH}" -d "${TEST_HOME}/docker" \ "${SCRIPT_DIR}/../../hugegraph-core/src/main/java/org/apache/hugegraph/config/AuthOptions.java" \ - "${SCRIPT_DIR}/test/AuthConfigReadback.java" + "${SCRIPT_DIR}/test/AuthConfigReadback.java" \ + "${SCRIPT_DIR}/test/ConfigReadback.java" else echo "SKIP: native auth readback (HG_CONFIG_TEST_CLASSPATH is unset)" fi @@ -91,6 +92,13 @@ assert_auth_readback() { fi } +assert_service_readback() { + if [[ -n "${HG_CONFIG_TEST_CLASSPATH:-}" ]]; then + java -cp "${TEST_HOME}/docker:${HG_CONFIG_TEST_CLASSPATH}" ConfigReadback \ + "${TEST_HOME}/conf/rest-server.properties" "$1" + fi +} + ( cd "${TEST_HOME}" HG_SERVER_BACKEND=hstore \ @@ -389,7 +397,10 @@ done [[ "$(cat "${TEST_HOME}/docker/init-store-locale")" == 'en_US.UTF-8' ]] # Identity mappings apply on every start and leave mounted config alone when unset. identity_conf="${TEST_HOME}/conf/rest-server.properties" -printf '\nserver.service_id=mounted-service\nserver.graphspace=mounted_space\n' >> "${identity_conf}" +# Earlier secret-escaping fixtures leave escaped trailing whitespace in this file. +# Start identity cases with their own parseable configuration. +printf 'restserver.url=http://127.0.0.1:8080\nserver.service_id=mounted-service\nserver.graphspace=mounted_space\n' \ + > "${identity_conf}" (cd "${TEST_HOME}"; bash ./docker-entrypoint.sh) grep -qx 'server.service_id=mounted-service' "${identity_conf}" grep -qx 'server.graphspace=mounted_space' "${identity_conf}" @@ -420,10 +431,20 @@ grep -qx 'server.graphspace=legacy_empty' "${identity_conf}" # Service permits spaces; Unicode escapes round-trip through the Server parser. (cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID='ops team' bash ./docker-entrypoint.sh) grep -Fxq 'server.service_id=ops\u0020team' "${identity_conf}" +assert_service_readback 'ops team' (cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID=' ops team ' bash ./docker-entrypoint.sh) grep -Fxq 'server.service_id=\u0020ops\u0020team\u0020' "${identity_conf}" +assert_service_readback ' ops team ' (cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID='ops\ team' bash ./docker-entrypoint.sh) grep -Fxq 'server.service_id=ops\\\u0020team' "${identity_conf}" +assert_service_readback 'ops\ team' + +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID=' ops\ team ' bash ./docker-entrypoint.sh) +assert_service_readback ' ops\ team ' +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID='ops\u0020team' bash ./docker-entrypoint.sh) +assert_service_readback 'ops\u0020team' +(cd "${TEST_HOME}"; HG_SERVER_SERVICE_ID='x-$(POD_IP)-$$(POD_IP)-$name-$' bash ./docker-entrypoint.sh) +assert_service_readback 'x-$(POD_IP)-$$(POD_IP)-$name-$' # Unsupported effective identity input must fail before changing mounted config. cp "${identity_conf}" "${identity_conf}.before-invalid-identity" diff --git a/hugegraph-server/hugegraph-dist/docker/test/ConfigReadback.java b/hugegraph-server/hugegraph-dist/docker/test/ConfigReadback.java new file mode 100644 index 0000000000..6c6b016bfc --- /dev/null +++ b/hugegraph-server/hugegraph-dist/docker/test/ConfigReadback.java @@ -0,0 +1,31 @@ +/* + * 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. + * + */ + +import org.apache.hugegraph.config.HugeConfig; + +public final class ConfigReadback { + + public static void main(String[] args) { + HugeConfig config = new HugeConfig(args[0]); + Object actual = config.getProperty("server.service_id"); + if (!args[1].equals(actual)) { + throw new AssertionError("Expected service ID [" + args[1] + "] but loaded [" + actual + "]"); + } + System.out.println("PASS: HugeConfig service ID readback [" + actual + "]"); + } +} From 7cc68c692282d23639349449eeff6a89ebf0e846 Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 15:23:56 +0800 Subject: [PATCH 04/11] fix(helm): select the graph checked by test - expose an explicit graph alias with existing defaults - retain authenticated per-Pod checks and retry limits - validate alias input and cover missing graph failures - explain target graph creation and binding requirements --- .github/workflows/helm-chart-ci.yml | 39 +++++++++++++++++-- helm/hugegraph/README.md | 22 +++++++++-- .../templates/tests/test-connection.yaml | 2 +- .../tests/test_hook_gremlin_test.yaml | 22 +++++++++++ helm/hugegraph/values.schema.json | 5 +++ helm/hugegraph/values.yaml | 3 ++ 6 files changed, 86 insertions(+), 7 deletions(-) diff --git a/.github/workflows/helm-chart-ci.yml b/.github/workflows/helm-chart-ci.yml index a01180729c..9cafc0256a 100644 --- a/.github/workflows/helm-chart-ci.yml +++ b/.github/workflows/helm-chart-ci.yml @@ -162,6 +162,16 @@ jobs: graph_space_max=$(printf 'a%.0s' {1..48}) helm template ci helm/hugegraph --set-string "server.graphSpace=${graph_space_max}" > /dev/null must_fail --set-string "server.graphSpace=${graph_space_max}a" + # The hook queries an explicit, fully qualified existing graph binding. + must_fail --set-string server.testGraphAlias= + must_fail --set-string server.testGraphAlias=team_a + must_fail --set-string server.testGraphAlias=team_a-demo-name + must_fail --set-string "server.testGraphAlias=team_a-demo'" + graph_name_max=$(printf 'g%.0s' {1..48}) + helm template ci helm/hugegraph \ + --set-string "server.testGraphAlias=${graph_space_max}-${graph_name_max}" > /dev/null + must_fail --set-string "server.testGraphAlias=${graph_space_max}a-demo" + must_fail --set-string "server.testGraphAlias=team_a-${graph_name_max}g" must_fail --set pd.replicas=100 must_fail --set pd.pdb.minAvailable=3 must_fail --set server.hpa.enabled=true @@ -344,7 +354,7 @@ jobs: printf '#!/bin/sh\nfor ip in ${STUB_IPS}; do printf "%%s STREAM host\\n" "$ip"; done\n' > /tmp/stub/getent chmod +x /tmp/stub/getent cat > /tmp/fixture.py <<'PY' - import http.server, os, sys, threading, time + import http.server, json, os, sys, threading, time calls = {} class H(http.server.BaseHTTPRequestHandler): def _send(self, code, body): @@ -356,7 +366,9 @@ jobs: def do_GET(self): self._send(200, b'{"versions":{}}') def do_POST(self): - self.rfile.read(int(self.headers.get('Content-Length', 0))) + body = json.loads(self.rfile.read(int(self.headers.get('Content-Length', 0)))) + expected_alias = ('team_a-demo' if os.path.exists('/tmp/selected-graph') + else 'DEFAULT-hugegraph') ip = self.server.server_address[0] # broken-: every Gremlin call fails. flip-odd- / # flip-even-: the odd or even calls fail, counted from @@ -366,6 +378,7 @@ jobs: even = os.path.exists('/tmp/flip-even-' + ip) n = calls[ip] = (calls.get(ip, 0) + 1) if (odd or even) else 0 broken = (os.path.exists('/tmp/broken-' + ip) + or body.get('aliases', {}).get('graph') != expected_alias or (odd and n % 2 == 1) or (even and n % 2 == 0)) if broken: @@ -383,7 +396,7 @@ jobs: sleep 1 run_hook() { PATH="/tmp/stub:${PATH}" PASSWORD=ci-password HG_TEST_DEADLINE_S=8 \ - STUB_IPS="$1" sh /tmp/hook.sh + STUB_IPS="$1" sh "${2:-/tmp/hook.sh}" } # Healthy: three Pods, all Gremlin-bound. run_hook "127.0.0.1 127.0.0.2 127.0.0.3" @@ -418,6 +431,26 @@ jobs: exit 1 fi run_hook "127.0.0.1 127.0.0.2 127.0.0.3 ::1 ::2 ::3" + # A non-default graph space must query the explicitly selected graph. + # A missing alias still fails rather than weakening the per-Pod sweep. + touch /tmp/selected-graph + for alias in team_a-demo team_a-missing; do + helm template ci helm/hugegraph -s templates/tests/test-connection.yaml \ + --set-string server.graphSpace=team_a \ + --set-string "server.testGraphAlias=${alias}" > "/tmp/hook-${alias}.yaml" + done + python3 - <<'PY' + import yaml + for alias in ('team_a-demo', 'team_a-missing'): + d = yaml.safe_load(open('/tmp/hook-' + alias + '.yaml')) + open('/tmp/hook-' + alias + '.sh', 'w').write(d['spec']['containers'][0]['command'][2]) + PY + run_hook "127.0.0.1 127.0.0.2 127.0.0.3" /tmp/hook-team_a-demo.sh + if run_hook "127.0.0.1 127.0.0.2 127.0.0.3" /tmp/hook-team_a-missing.sh; then + echo "expected the hook to fail on a missing selected graph" >&2 + exit 1 + fi + rm /tmp/selected-graph # The auth-off render must carry no credentials. if helm template ci helm/hugegraph -s templates/tests/test-connection.yaml \ --set server.auth.enabled=false | grep -q 'Authorization'; then diff --git a/helm/hugegraph/README.md b/helm/hugegraph/README.md index 558ba906ef..62953d3212 100644 --- a/helm/hugegraph/README.md +++ b/helm/hugegraph/README.md @@ -202,7 +202,10 @@ or recovery. Hubble health probes and the Server hook do not verify UI access. Besides the Service checks, the test resolves a headless Server Service and requires an authenticated, graph-bound Gremlin answer from every Server Pod -(at least the replica floor), because a Server that started during a PD roll +(at least the replica floor). `server.testGraphAlias` selects that binding and +defaults to `DEFAULT-hugegraph`. Set it to an existing target graph's full alias, +such as `team_a-demo`, to check that graph instead. The hook does not create a +graph or infer an alias from `server.graphSpace`. A Server that started during a PD roll can pass readiness and serve REST while every Gremlin call on it fails (see Troubleshooting, "Could not rebind"). Transient failures are retried for 150 seconds before the test fails and names the failing Pod IPs. Large HPA @@ -617,6 +620,7 @@ default values. | `server.probes.startup.failureThreshold` | Raised automatically so the guaranteed budget, `(failureThreshold - 1) * periodSeconds` (the first probe can fail immediately), is at least 450s | `91` | | `server.probes.startup.periodSeconds` | Startup probe interval | `5` | | `server.probes.*` | Same optional probe keys as PD | see `values.yaml` | +| `server.testGraphAlias` | Existing Gremlin graph binding checked on every Server Pod by `helm test` | `DEFAULT-hugegraph` | When `server.hpa.enabled` is `true` the Deployment omits `spec.replicas`, so a Helm upgrade does not overwrite the autoscaler's live replica count. Enabling @@ -653,6 +657,13 @@ existing graphs or change `server.path_graphspace`. Use explicit Hubble. Rendering and entrypoint checks verify configuration only; non-default PD discovery, authentication and isolation require a running Linux cluster. +To validate a graph in the selected space, create the graph there and set +`server.testGraphAlias` to its actual Gremlin binding, for example +`--set-string server.testGraphAlias=team_a-demo`. Creating `team_a` does not +create a `hugegraph` graph. The default hook checks the local `DEFAULT-hugegraph` +binding, which may exist independently of the Server's assigned PD graph space; +its success alone does not validate graphs in `team_a`. + ### Reaching Hubble (pick one path) Most people should stop at **1**. Use **2** only if Hubble must run outside @@ -1093,8 +1104,13 @@ curl -s --compressed -u "admin:${PASSWORD}" -H 'Content-Type: application/json' -d '{"gremlin":"graph.traversal().V().limit(1).count()","aliases":{"graph":"DEFAULT-hugegraph"}}' ``` -A healthy Pod answers with `result.data`; delete a Pod that answers -`Could not rebind`, and its replacement binds normally once PD is stable. +A healthy Pod answers with `result.data`. This example uses the default +`server.testGraphAlias`; replace `DEFAULT-hugegraph` with your configured alias. +If a Pod answers `Could not rebind`, first confirm that the requested graph +exists and the alias matches its binding. For a missing binding caused by the +PD startup failure above, delete the affected Pod once PD is stable so its +replacement can bind the graph. Restarting a Pod cannot create a missing graph +or correct the requested alias. `helm test` runs this same graph-bound query against every Server Pod through the headless Service, so a stuck replica now fails the release check instead of hiding behind the load balancer. The measurements behind diff --git a/helm/hugegraph/templates/tests/test-connection.yaml b/helm/hugegraph/templates/tests/test-connection.yaml index 7d71d323ba..ea17f709e7 100644 --- a/helm/hugegraph/templates/tests/test-connection.yaml +++ b/helm/hugegraph/templates/tests/test-connection.yaml @@ -74,7 +74,7 @@ spec: PORT="{{ .Values.server.port }}" FLOOR="{{ include "hugegraph.server.replicaFloor" . }}" DEADLINE_S="${HG_TEST_DEADLINE_S:-150}" - BODY='{"gremlin":"graph.traversal().V().limit(1).count()","aliases":{"graph":"DEFAULT-hugegraph"}}' + BODY={{ dict "gremlin" "graph.traversal().V().limit(1).count()" "aliases" (dict "graph" (.Values.server.testGraphAlias | default "DEFAULT-hugegraph")) | toJson | squote }} {{- if .Values.server.auth.enabled }} AUTH_HEADER=$(printf 'admin:%s' "${PASSWORD}" | base64 | tr -d '\r\n') request() { diff --git a/helm/hugegraph/tests/test_hook_gremlin_test.yaml b/helm/hugegraph/tests/test_hook_gremlin_test.yaml index acd41bf25b..09c1df9141 100644 --- a/helm/hugegraph/tests/test_hook_gremlin_test.yaml +++ b/helm/hugegraph/tests/test_hook_gremlin_test.yaml @@ -60,6 +60,28 @@ tests: path: spec.containers[0].command[2] pattern: "FLOOR=\"3\"" + - it: checks the explicitly selected graph in a non-default graph space + template: tests/test-connection.yaml + set: + server.graphSpace: team_a + server.testGraphAlias: team_a-demo + asserts: + - matchRegex: + path: spec.containers[0].command[2] + pattern: '"graph":"team_a-demo"' + - notMatchRegex: + path: spec.containers[0].command[2] + pattern: "DEFAULT-hugegraph" + + - it: preserves the default binding when graphSpace changes without a selected graph + template: tests/test-connection.yaml + set: + server.graphSpace: team_a + asserts: + - matchRegex: + path: spec.containers[0].command[2] + pattern: '"graph":"DEFAULT-hugegraph"' + - it: floors the per-Pod count at the HPA minimum when HPA is enabled template: tests/test-connection.yaml set: diff --git a/helm/hugegraph/values.schema.json b/helm/hugegraph/values.schema.json index 6770e10db2..6eef295e2c 100644 --- a/helm/hugegraph/values.schema.json +++ b/helm/hugegraph/values.schema.json @@ -774,6 +774,11 @@ "pattern": "^(DEFAULT|[a-z][a-z0-9_]{0,47})$", "description": "Existing PD graph space: DEFAULT or up to 48 lowercase letters, digits and underscores, starting with a letter. DEFAULT preserves the image config." }, + "testGraphAlias": { + "type": "string", + "pattern": "^(DEFAULT|[a-z][a-z0-9_]{0,47})-[A-Za-z][A-Za-z0-9_]{0,47}$", + "description": "Existing Gremlin graph binding checked by helm test, as -. Choose the target graph explicitly; server.graphSpace does not create or infer this binding." + }, "replicas": { "type": "integer", "minimum": 1 diff --git a/helm/hugegraph/values.yaml b/helm/hugegraph/values.yaml index b99ef12d8d..a1eb760019 100644 --- a/helm/hugegraph/values.yaml +++ b/helm/hugegraph/values.yaml @@ -336,6 +336,9 @@ server: minAvailable: 2 # Image used by the Helm test hook. waitImage: curlimages/curl:8.5.0 + # Existing Gremlin graph binding checked on every Server Pod. Select the + # target graph explicitly; graphSpace does not create or infer this binding. + testGraphAlias: DEFAULT-hugegraph # Resources for the Helm test hook container. Bounded by default so the hook # cannot run unlimited on a restricted or quota-managed namespace. testResources: From fd04299b99f2518b7e2f2a08fecef8e9815f8586 Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 23:36:20 +0800 Subject: [PATCH 05/11] fix(server): retain full service names - preserve complete names when reloading service metadata - keep the service suffix when clearing a graph space - cover metadata write, reload, removal and space clearing - document the case of existing Gremlin graph bindings --- docker/README.md | 2 + helm/hugegraph/README.md | 9 +- .../tests/test_hook_gremlin_test.yaml | 9 + .../apache/hugegraph/core/GraphManager.java | 2 +- .../meta/managers/ServiceMetaManager.java | 7 +- .../apache/hugegraph/unit/UnitTestSuite.java | 2 + .../core/GraphManagerServiceNamesTest.java | 155 ++++++++++++++++++ 7 files changed, 179 insertions(+), 7 deletions(-) create mode 100644 hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerServiceNamesTest.java diff --git a/docker/README.md b/docker/README.md index 7c08def43a..82d802f2fd 100644 --- a/docker/README.md +++ b/docker/README.md @@ -283,6 +283,8 @@ spaces and backslashes in service IDs round-trip unchanged. Non-ASCII and contro characters are rejected before the entrypoint writes configuration, because the Server reads properties using ISO-8859-1. This is the container input's serialization limit; it does not change the core service-name contract. +The full service ID, including `/` and `-`, is retained when service metadata is +reloaded or its graph space is cleared. `HG_SERVER_GRAPHSPACE` and a migrated `GRAPH_SPACE` must also match the Server API graph-space rule: `DEFAULT`, or up to 48 lowercase letters, digits and underscores, starting with a letter. Invalid environment values are rejected diff --git a/helm/hugegraph/README.md b/helm/hugegraph/README.md index 62953d3212..56aba35d91 100644 --- a/helm/hugegraph/README.md +++ b/helm/hugegraph/README.md @@ -205,7 +205,9 @@ requires an authenticated, graph-bound Gremlin answer from every Server Pod (at least the replica floor). `server.testGraphAlias` selects that binding and defaults to `DEFAULT-hugegraph`. Set it to an existing target graph's full alias, such as `team_a-demo`, to check that graph instead. The hook does not create a -graph or infer an alias from `server.graphSpace`. A Server that started during a PD roll +graph or infer an alias from `server.graphSpace`. Match the binding's case: a +local `Demo.properties` can bind `DEFAULT-Demo`, while REST-created graph names +are lowercase. A Server that started during a PD roll can pass readiness and serve REST while every Gremlin call on it fails (see Troubleshooting, "Could not rebind"). Transient failures are retried for 150 seconds before the test fails and names the failing Pod IPs. Large HPA @@ -639,7 +641,10 @@ and backslashes. Dollar signs are escaped in the manifest so Kubernetes passes the service ID literally, including `$(POD_IP)` and `$$`. The chart and entrypoint reject non-ASCII and control characters to prevent the Server's ISO-8859-1 properties reader from changing the value; -this is a deployment input serialization limit, not a change to core service names: +this is a deployment input serialization limit, not a change to core service names. + +The full service ID, including `/` and `-`, is retained when service metadata is +reloaded or its graph space is cleared. ```bash helm upgrade --install hg ./helm/hugegraph \ diff --git a/helm/hugegraph/tests/test_hook_gremlin_test.yaml b/helm/hugegraph/tests/test_hook_gremlin_test.yaml index 09c1df9141..41deb39cc2 100644 --- a/helm/hugegraph/tests/test_hook_gremlin_test.yaml +++ b/helm/hugegraph/tests/test_hook_gremlin_test.yaml @@ -82,6 +82,15 @@ tests: path: spec.containers[0].command[2] pattern: '"graph":"DEFAULT-hugegraph"' + - it: preserves a static graph binding with an uppercase local file name + template: tests/test-connection.yaml + set: + server.testGraphAlias: DEFAULT-Demo + asserts: + - matchRegex: + path: spec.containers[0].command[2] + pattern: '"graph":"DEFAULT-Demo"' + - it: floors the per-Pod count at the HPA minimum when HPA is enabled template: tests/test-connection.yaml set: diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index c7d821bf38..bd66a0343f 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -1124,7 +1124,7 @@ public void clearGraphSpace(String name) { // Clear all services for (String key : this.services.keySet()) { if (key.startsWith(name + DELIMITER)) { - String[] parts = key.split(DELIMITER); + String[] parts = key.split(DELIMITER, 2); this.dropService(parts[0], parts[1]); } } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/meta/managers/ServiceMetaManager.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/meta/managers/ServiceMetaManager.java index 3c03be3433..14a453aef8 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/meta/managers/ServiceMetaManager.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/meta/managers/ServiceMetaManager.java @@ -50,12 +50,11 @@ private static String serviceName(String graphSpace, String name) { public Map serviceConfigs(String graphSpace) { Map serviceMap = new HashMap<>(); - Map keyValues = this.metaDriver.scanWithPrefix( - this.serviceConfPrefix(graphSpace)); + String prefix = this.serviceConfPrefix(graphSpace); + Map keyValues = this.metaDriver.scanWithPrefix(prefix); for (Map.Entry entry : keyValues.entrySet()) { String key = entry.getKey(); - String[] parts = key.split(META_PATH_DELIMITER); - serviceMap.put(parts[parts.length - 1], + serviceMap.put(key.substring(prefix.length()), JsonUtil.fromJson(entry.getValue(), Service.class)); } return serviceMap; diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java index 3cd65b0d8f..0cea17b5d4 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java @@ -76,6 +76,7 @@ import org.apache.hugegraph.unit.core.ExceptionTest; import org.apache.hugegraph.unit.core.GraphManagerAdminInitTest; import org.apache.hugegraph.unit.core.GraphManagerConfigTest; +import org.apache.hugegraph.unit.core.GraphManagerServiceNamesTest; import org.apache.hugegraph.unit.core.GroovyScriptEngineCompatibilityTest; import org.apache.hugegraph.unit.core.HugeFeaturesTest; import org.apache.hugegraph.unit.core.HstoreSessionsTest; @@ -216,6 +217,7 @@ ExceptionTest.class, GraphManagerAdminInitTest.class, GraphManagerConfigTest.class, + GraphManagerServiceNamesTest.class, HstoreSessionsTest.class, HstoreTableRoutingTest.class, BackendStoreInfoTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerServiceNamesTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerServiceNamesTest.java new file mode 100644 index 0000000000..fa02979daa --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerServiceNamesTest.java @@ -0,0 +1,155 @@ +/* + * 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. + */ + +package org.apache.hugegraph.unit.core; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.stream.Collectors; + +import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.core.GraphManager; +import org.apache.hugegraph.event.EventHub; +import org.apache.hugegraph.meta.MetaDriver; +import org.apache.hugegraph.meta.MetaManager; +import org.apache.hugegraph.meta.lock.LockResult; +import org.apache.hugegraph.meta.managers.ServiceMetaManager; +import org.apache.hugegraph.space.GraphSpace; +import org.apache.hugegraph.space.Service; +import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.testutil.Whitebox; +import org.junit.Test; +import org.mockito.Mockito; + +public class GraphManagerServiceNamesTest { + + @Test + public void testReloadPreservesCompleteServiceNames() { + MetaDriver driver = storageDriver(); + ServiceMetaManager first = new ServiceMetaManager(driver, "cluster"); + for (String name : new String[]{"analytics", "team/analytics", + "analytics-prod", "team/analytics-prod"}) { + first.addServiceConfig("team_a", service(name)); + } + first.addServiceConfig("other", service("analytics")); + + ServiceMetaManager reloaded = new ServiceMetaManager(driver, "cluster"); + Map services = reloaded.serviceConfigs("team_a"); + Assert.assertEquals(4, services.size()); + for (String name : new String[]{"analytics", "team/analytics", + "analytics-prod", "team/analytics-prod"}) { + Assert.assertEquals(name, services.get(name).name()); + Assert.assertEquals(name, reloaded.service("team_a", name).name()); + } + Assert.assertEquals(1, reloaded.serviceConfigs("other").size()); + + reloaded.removeServiceConfig("team_a", "team/analytics"); + Assert.assertNull(reloaded.service("team_a", "team/analytics")); + Assert.assertNotNull(reloaded.service("team_a", "analytics")); + Assert.assertEquals(3, reloaded.serviceConfigs("team_a").size()); + } + + @Test + public void testClearSpaceDropsFullServiceNames() throws Exception { + MetaDriver driver = storageDriver(); + Constructor constructor = MetaManager.class.getDeclaredConstructor(); + constructor.setAccessible(true); + MetaManager metadata = constructor.newInstance(); + setField(metadata, "metaDriver", driver); + setField(metadata, "cluster", "cluster"); + Whitebox.invoke(MetaManager.class, new Class[]{String.class}, + "initManagers", metadata, "cluster"); + GraphManager manager = new GraphManager( + new HugeConfig(new PropertiesConfiguration()), + new EventHub("service-name-test")); + try { + setField(manager, "metaManager", metadata); + // Exercise the metadata-backed clear path without bootstrapping + // a PD/HStore cluster; the real managers use a storage-only seam. + setField(manager, "PDExist", true); + Map spaces = Whitebox.getInternalState(manager, "graphSpaces"); + spaces.put("team_a", new GraphSpace("team_a")); + spaces.put("other", new GraphSpace("other")); + Map cache = Whitebox.getInternalState(manager, "services"); + for (String name : new String[]{"analytics", "analytics-prod", + "team/analytics-prod"}) { + Service service = service(name); + metadata.addServiceConfig("team_a", service); + String key = Whitebox.invoke(GraphManager.class, + new Class[]{String.class, String.class}, "serviceName", + null, "team_a", name); + cache.put(key, service); + } + metadata.addServiceConfig("other", service("analytics")); + cache.put("other-analytics", service("analytics")); + + manager.clearGraphSpace("team_a"); + + Assert.assertTrue(metadata.serviceConfigs("team_a").isEmpty()); + Assert.assertEquals(1, metadata.serviceConfigs("other").size()); + Assert.assertEquals(1, cache.size()); + Assert.assertTrue(cache.containsKey("other-analytics")); + } finally { + manager.close(); + } + } + + private static Service service(String name) { + return new Service(name, "admin", Service.ServiceType.OLTP, + Service.DeploymentType.MANUAL); + } + + private static void setField(Object target, String name, Object value) + throws Exception { + Field field = target.getClass().getDeclaredField(name); + field.setAccessible(true); + field.set(target, value); + } + + private static MetaDriver storageDriver() { + Map storage = new LinkedHashMap<>(); + MetaDriver driver = Mockito.mock(MetaDriver.class); + Mockito.doAnswer(call -> { + storage.put(call.getArgument(0), call.getArgument(1)); + return null; + }).when(driver).put(Mockito.anyString(), Mockito.anyString()); + Mockito.when(driver.get(Mockito.anyString())) + .thenAnswer(call -> storage.get(call.getArgument(0))); + Mockito.doAnswer(call -> { + storage.remove(call.getArgument(0)); + return null; + }).when(driver).delete(Mockito.anyString()); + Mockito.doAnswer(call -> { + storage.keySet().removeIf(key -> key.startsWith(call.getArgument(0))); + return null; + }).when(driver).deleteWithPrefix(Mockito.anyString()); + Mockito.when(driver.scanWithPrefix(Mockito.anyString())).thenAnswer(call -> + storage.entrySet().stream() + .filter(entry -> entry.getKey().startsWith(call.getArgument(0))) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))); + Mockito.when(driver.tryLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyLong())) + .thenAnswer(call -> { + LockResult lock = new LockResult(); + lock.lockSuccess(true); + return lock; + }); + return driver; + } +} From 2241dfdc0c6a107617abe2a15dafe9d2a51cf2a3 Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 14 Oct 2026 04:23:27 +0800 Subject: [PATCH 06/11] fix(docker): retain discovery test ordering - retain service identity readback and original test fixtures - preserve upstream authentication regression coverage - keep consolidated Docker CI runner and contract classpath --- .../docker/docker-entrypoint-test.sh | 84 +++++++++---------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh index bb88d06739..38336599b7 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh @@ -75,8 +75,14 @@ if [[ -n "${HG_CONFIG_TEST_CLASSPATH:-}" ]]; then "${SCRIPT_DIR}/test/AuthConfigReadback.java" \ "${SCRIPT_DIR}/test/ConfigReadback.java" else - echo "SKIP: native auth readback (HG_CONFIG_TEST_CLASSPATH is unset)" + echo "SKIP: native auth and service readback (HG_CONFIG_TEST_CLASSPATH is unset)" fi +assert_service_readback() { + if [[ -n "${HG_CONFIG_TEST_CLASSPATH:-}" ]]; then + java -cp "${TEST_HOME}/docker:${HG_CONFIG_TEST_CLASSPATH}" ConfigReadback \ + "${TEST_HOME}/conf/rest-server.properties" "$1" + fi +} assert_auth_readback() { if [[ -n "${HG_CONFIG_TEST_CLASSPATH:-}" ]]; then local secret_base64 admin_base64 @@ -92,13 +98,6 @@ assert_auth_readback() { fi } -assert_service_readback() { - if [[ -n "${HG_CONFIG_TEST_CLASSPATH:-}" ]]; then - java -cp "${TEST_HOME}/docker:${HG_CONFIG_TEST_CLASSPATH}" ConfigReadback \ - "${TEST_HOME}/conf/rest-server.properties" "$1" - fi -} - ( cd "${TEST_HOME}" HG_SERVER_BACKEND=hstore \ @@ -361,40 +360,6 @@ done ) assert_start_timeout 120 -# Signing keys are opaque strings: both files must load exactly what the caller supplied. -fake_key='FAKE_SIGNING_KEY_0123456789abcdef' -control_soh=$(printf '\001') -control_us=$(printf '\037') -min_key="${fake_key:0:31}${control_soh}" -[[ ${#min_key} -eq 32 ]] -auth_secrets=("${fake_key}" " ${fake_key}" "${fake_key} inner" "${fake_key} " - "${fake_key}"$'\t' "${fake_key}\\" "${fake_key}café" "${fake_key}中文" "${fake_key}😀" - "${fake_key}\\u0020😀 café " "${fake_key}"$'\377' - "${control_soh}${fake_key}" "${fake_key}${control_soh}" "${fake_key}"$'\v' "${fake_key}${control_us}" - "${min_key}") -fake_admin_password="${control_soh}"' fake\ admin '"${control_us}" -for auth_secret in "${auth_secrets[@]}"; do - cp "${SCRIPT_DIR}/../src/assembly/static/conf/rest-server.properties" \ - "${TEST_HOME}/conf/rest-server.properties" - (cd "${TEST_HOME}"; PASSWORD="${fake_admin_password}" HG_SERVER_AUTH_TOKEN_SECRET="${auth_secret}" \ - bash ./docker-entrypoint.sh) - expected_secret="${auth_secret}" - if [[ "${auth_secret}" == "${fake_key}"$'\377' ]]; then - expected_secret="${fake_key}ÿ" - fi - assert_auth_readback "${TEST_HOME}/conf/rest-server.properties" "${expected_secret}" "${fake_admin_password}" - assert_auth_readback "${TEST_HOME}/conf/graphs/hugegraph.properties" "${expected_secret}" - cp "${TEST_HOME}/conf/rest-server.properties" "${TEST_HOME}/conf/rest-server.properties.before-reuse" - cp "${TEST_HOME}/conf/graphs/hugegraph.properties" "${TEST_HOME}/conf/graphs/hugegraph.properties.before-reuse" - (cd "${TEST_HOME}"; PASSWORD="${fake_admin_password}" bash ./docker-entrypoint.sh) - cmp "${TEST_HOME}/conf/rest-server.properties.before-reuse" "${TEST_HOME}/conf/rest-server.properties" - cmp "${TEST_HOME}/conf/graphs/hugegraph.properties.before-reuse" "${TEST_HOME}/conf/graphs/hugegraph.properties" - assert_auth_readback "${TEST_HOME}/conf/rest-server.properties" "${expected_secret}" "${fake_admin_password}" -done - -(cd "${TEST_HOME}"; LC_ALL=en_US.UTF-8 PASSWORD='locale probe' \ - HG_SERVER_AUTH_TOKEN_SECRET="${fake_key}" bash ./docker-entrypoint.sh) -[[ "$(cat "${TEST_HOME}/docker/init-store-locale")" == 'en_US.UTF-8' ]] # Identity mappings apply on every start and leave mounted config alone when unset. identity_conf="${TEST_HOME}/conf/rest-server.properties" # Earlier secret-escaping fixtures leave escaped trailing whitespace in this file. @@ -505,5 +470,40 @@ for identity_locale in sk_SK.ISO8859-2 cs_CZ.ISO8859-2; do done done +# Signing keys are opaque strings: both files must load exactly what the caller supplied. +fake_key='FAKE_SIGNING_KEY_0123456789abcdef' +control_soh=$(printf '\001') +control_us=$(printf '\037') +min_key="${fake_key:0:31}${control_soh}" +[[ ${#min_key} -eq 32 ]] +auth_secrets=("${fake_key}" " ${fake_key}" "${fake_key} inner" "${fake_key} " + "${fake_key}"$'\t' "${fake_key}\\" "${fake_key}café" "${fake_key}中文" "${fake_key}😀" + "${fake_key}\\u0020😀 café " "${fake_key}"$'\377' + "${control_soh}${fake_key}" "${fake_key}${control_soh}" "${fake_key}"$'\v' "${fake_key}${control_us}" + "${min_key}") +fake_admin_password="${control_soh}"' fake\ admin '"${control_us}" +for auth_secret in "${auth_secrets[@]}"; do + cp "${SCRIPT_DIR}/../src/assembly/static/conf/rest-server.properties" \ + "${TEST_HOME}/conf/rest-server.properties" + (cd "${TEST_HOME}"; PASSWORD="${fake_admin_password}" HG_SERVER_AUTH_TOKEN_SECRET="${auth_secret}" \ + bash ./docker-entrypoint.sh) + expected_secret="${auth_secret}" + if [[ "${auth_secret}" == "${fake_key}"$'\377' ]]; then + expected_secret="${fake_key}ÿ" + fi + assert_auth_readback "${TEST_HOME}/conf/rest-server.properties" "${expected_secret}" "${fake_admin_password}" + assert_auth_readback "${TEST_HOME}/conf/graphs/hugegraph.properties" "${expected_secret}" + cp "${TEST_HOME}/conf/rest-server.properties" "${TEST_HOME}/conf/rest-server.properties.before-reuse" + cp "${TEST_HOME}/conf/graphs/hugegraph.properties" "${TEST_HOME}/conf/graphs/hugegraph.properties.before-reuse" + (cd "${TEST_HOME}"; PASSWORD="${fake_admin_password}" bash ./docker-entrypoint.sh) + cmp "${TEST_HOME}/conf/rest-server.properties.before-reuse" "${TEST_HOME}/conf/rest-server.properties" + cmp "${TEST_HOME}/conf/graphs/hugegraph.properties.before-reuse" "${TEST_HOME}/conf/graphs/hugegraph.properties" + assert_auth_readback "${TEST_HOME}/conf/rest-server.properties" "${expected_secret}" "${fake_admin_password}" +done + +(cd "${TEST_HOME}"; LC_ALL=en_US.UTF-8 PASSWORD='locale probe' \ + HG_SERVER_AUTH_TOKEN_SECRET="${fake_key}" bash ./docker-entrypoint.sh) +[[ "$(cat "${TEST_HOME}/docker/init-store-locale")" == 'en_US.UTF-8' ]] + echo "PASS: Docker entrypoint configures HStore discovery and authentication" bash "${SCRIPT_DIR}/test/test-docker-entrypoint-process.sh" From 670eb720696f55e62c75250ac2b458b2d27ef4c0 Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 14 Oct 2026 14:02:08 +0800 Subject: [PATCH 07/11] fix(pd): reject discovery identity collisions - compare raw service tuples before registration - fence discovery writes with the current Raft term - reject ambiguous service pools during discovery - require compatible image identity mapping in Helm - cover registry and pinned-image behavior --- helm/hugegraph/README.md | 20 ++- .../templates/server-deployment.yaml | 15 ++ .../tests/server_discovery_test.yaml | 6 + helm/test_helm_package.py | 72 ++++++++ hugegraph-pd/docs/api-reference.md | 28 +++ .../apache/hugegraph/pd/RegistryService.java | 84 ++++++++- .../hugegraph/pd/meta/DiscoveryMetaStore.java | 39 ++++ .../apache/hugegraph/pd/raft/RaftEngine.java | 34 ++++ .../hugegraph/pd/raft/RaftStateMachine.java | 4 + .../hugegraph/pd/store/RaftKVStore.java | 15 ++ .../pd/service/DiscoveryService.java | 9 +- .../hugegraph/pd/service/PDRestService.java | 13 +- .../hugegraph/pd/core/PDCoreSuiteTest.java | 1 + .../pd/core/RegistryIdentityTest.java | 166 ++++++++++++++++++ .../apache/hugegraph/core/GraphManager.java | 6 +- 15 files changed, 498 insertions(+), 14 deletions(-) create mode 100644 hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RegistryIdentityTest.java diff --git a/helm/hugegraph/README.md b/helm/hugegraph/README.md index 56aba35d91..7acc49dce0 100644 --- a/helm/hugegraph/README.md +++ b/helm/hugegraph/README.md @@ -646,13 +646,31 @@ this is a deployment input serialization limit, not a change to core service nam The full service ID, including `/` and `-`, is retained when service metadata is reloaded or its graph space is cleared. +PD protects a live discovery pool when its registrations provide the complete +`GRAPHSPACE`/`SERVICE_NAME`/`SERVICE_ID` tuple. Different raw names that map to the +same discovery identifier are rejected rather than merged into one endpoint +pool. The configured names and printable-character rules above are unchanged. +Upgrade PD first; old PD leaders do not provide this check. Registration is +asynchronous, so a conflicting Server can remain running locally while logging +a registration error. Stop conflicting heartbeats and wait for their TTL, or +explicitly select a different service identity. A generic-only pool must also +expire before a complete Server identity can join it. Clients should select the +raw graph space and service name; identifier-only queries to generic-only pools +keep their previous behavior. + ```bash helm upgrade --install hg ./helm/hugegraph \ --set-string server.serviceId=analytics \ --set-string server.graphSpace=team_a ``` -Use images built with these entrypoint mappings. Create the non-default graph +For non-default identities, the startup wrapper checks the image entrypoint's +environment-to-property mappings before rewriting configuration or launching +Server. An older or unrecognized entrypoint stops with a `compatible Server +image` error instead of silently using `DEFAULT`. Select an image built with +these mappings; the chart's default image tag is unchanged. + +Create the non-default graph space in the existing cluster before deploying Servers assigned to it; the chart does not create it. A graph-space name is `DEFAULT` or up to 48 lowercase letters, digits and underscores, starting with a letter, as required by the Server API. All replicas in this Server Deployment use the same identity. diff --git a/helm/hugegraph/templates/server-deployment.yaml b/helm/hugegraph/templates/server-deployment.yaml index db66db59d4..3f2c5bad95 100644 --- a/helm/hugegraph/templates/server-deployment.yaml +++ b/helm/hugegraph/templates/server-deployment.yaml @@ -158,6 +158,21 @@ spec: exit 1 fi {{- end }} + # Older images ignore these optional identity variables. Require + # their actual env-to-property mappings before changing config. + if [[ -n "${HG_SERVER_SERVICE_ID:-}" ]]; then + if ! grep -Eq '^[[:space:]]*set_prop[[:space:]]+"server\.service_id"[[:space:]]+"\$\{HG_SERVER_SERVICE_ID\}"[[:space:]]+"\$\{REST_SERVER_CONF\}"([[:space:]]|$)' ./docker-entrypoint.sh && + ! (grep -Eq '^[[:space:]]*service_id_encoded=\$\(encode_prop_value[[:space:]]+"\$\{HG_SERVER_SERVICE_ID\}"' ./docker-entrypoint.sh && + grep -Eq '^[[:space:]]*set_prop_encoded[[:space:]]+"server\.service_id"[[:space:]]+"\$\{service_id_encoded\}"[[:space:]]+"\$\{REST_SERVER_CONF\}"([[:space:]]|$)' ./docker-entrypoint.sh); then + echo "non-default server.serviceId requires a compatible Server image with the HG_SERVER_SERVICE_ID mapping" >&2 + exit 1 + fi + fi + if [[ -n "${HG_SERVER_GRAPHSPACE:-}" ]] && + ! grep -Eq '^[[:space:]]*set_prop[[:space:]]+"server\.graphspace"[[:space:]]+"\$\{HG_SERVER_GRAPHSPACE\}"[[:space:]]+"\$\{REST_SERVER_CONF\}"([[:space:]]|$)' ./docker-entrypoint.sh; then + echo "non-default server.graphSpace requires a compatible Server image with the HG_SERVER_GRAPHSPACE mapping" >&2 + exit 1 + fi CONF=./conf/rest-server.properties if [[ ! -r "${CONF}" ]]; then echo "missing or unreadable ${CONF}; refusing to write a stub config" >&2 diff --git a/helm/hugegraph/tests/server_discovery_test.yaml b/helm/hugegraph/tests/server_discovery_test.yaml index 92d5adfaaa..cd9b9a3d1d 100644 --- a/helm/hugegraph/tests/server_discovery_test.yaml +++ b/helm/hugegraph/tests/server_discovery_test.yaml @@ -66,6 +66,12 @@ tests: - equal: path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_GRAPHSPACE")].value value: team_a + - matchRegex: + path: spec.template.spec.containers[0].args[0] + pattern: 'non-default server\.serviceId requires a compatible Server image' + - matchRegex: + path: spec.template.spec.containers[0].args[0] + pattern: 'non-default server\.graphSpace requires a compatible Server image' - it: refuses an extraEnv that shadows the service ID set: diff --git a/helm/test_helm_package.py b/helm/test_helm_package.py index ad43d5d399..0e3896b0bb 100644 --- a/helm/test_helm_package.py +++ b/helm/test_helm_package.py @@ -17,6 +17,7 @@ """Validate the artifact shipped to Helm, beyond source template tests.""" from pathlib import Path +import os import shutil import subprocess import tarfile @@ -94,6 +95,77 @@ def test_packaged_chart_renders_each_preset(self): self.assertTrue(b"kind: Deployment" in rendered.stdout, "Packaged chart did not render a Deployment") + def test_service_identity_requires_image_entrypoint_mappings(self): + command = ["helm", "template", "identity-check", str(self.package), + "--show-only", "templates/server-deployment.yaml", + "--set", "server.auth.enabled=false"] + rendered = subprocess.run(command, check=True, capture_output=True, text=True).stdout + # Read the literal shell block without adding a YAML dependency. + lines = rendered.splitlines() + start = lines.index(" args:") + 2 + shell = [] + for line in lines[start:]: + if line.strip() and not line.startswith(" "): + break + shell.append(line[14:]) + script = "\n".join(shell) + "\n" + self.assertTrue(script.startswith("set -euo pipefail\n")) + original = "server.service_id=DEFAULT\nserver.graphspace=DEFAULT\nusePD=false\n" + old = "#!/bin/bash\nprintf launched > launched\n" + current = (ROOT / "hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh").read_text() + # Execute the actual image's validation and mappings, then stop before + # storage initialization or Java launch. No properties encoder is copied. + compatible = current.split("# ── Build wait-storage env", 1)[0] + "\nprintf launched > launched\n" + comment_only = old + "\n".join("# " + line for line in current.splitlines()) + "\n" + with tempfile.TemporaryDirectory(prefix="hugegraph-identity-wrapper-") as temporary: + directory = Path(temporary) + (directory / "conf/graphs").mkdir(parents=True) + (directory / "conf/graphs/hugegraph.properties").write_text("backend=hstore\n") + # The image mapping uses GNU sed. Reuse it when running on macOS; + # Linux CI already provides it as sed. + executable_path = os.environ["PATH"] + if shutil.which("gsed"): + (directory / "bin").mkdir() + (directory / "bin/sed").symlink_to(shutil.which("gsed")) + executable_path = str(directory / "bin") + os.pathsep + executable_path + config = directory / "conf/rest-server.properties" + entrypoint = directory / "docker-entrypoint.sh" + launcher = directory / "wrapper.sh" + launcher.write_text(script) + for name, image, identity, supported in ( + ("old service", old, {"HG_SERVER_SERVICE_ID": "analytics"}, False), + ("old graph space", old, {"HG_SERVER_GRAPHSPACE": "team_a"}, False), + ("comment-only service", comment_only, {"HG_SERVER_SERVICE_ID": "analytics"}, False), + ("comment-only graph space", comment_only, {"HG_SERVER_GRAPHSPACE": "team_a"}, False), + ("current mappings", compatible, + {"HG_SERVER_SERVICE_ID": "analytics", "HG_SERVER_GRAPHSPACE": "team_a"}, True), + ("default old image", old, {}, True)): + with self.subTest(case=name): + config.write_text(original) + entrypoint.write_text(image) + entrypoint.chmod(0o700) + (directory / "launched").unlink(missing_ok=True) + environment = {"PATH": executable_path, + "HG_SERVER_PD_PEERS": "fixture:8686", + "HG_SERVER_URLS_TO_PD": "http://fixture:8080", **identity} + run = subprocess.run(["bash", str(launcher)], cwd=directory, env=environment, + capture_output=True, text=True, timeout=10) + if supported: + self.assertEqual(0, run.returncode, run.stderr) + self.assertTrue((directory / "launched").exists()) + self.assertIn("usePD=true\n", config.read_text()) + if identity: + self.assertIn("server.service_id=analytics\n", config.read_text()) + self.assertIn("server.graphspace=team_a\n", config.read_text()) + else: + self.assertNotEqual(0, run.returncode, + "Unsupported image launched=" + + str((directory / "launched").exists()) + + "; config changed=" + str(config.read_text() != original)) + self.assertIn("compatible Server image", run.stderr) + self.assertEqual(original, config.read_text(), "Rejected image changed config") + self.assertFalse((directory / "launched").exists(), "Rejected image was launched") + if __name__ == "__main__": unittest.main() diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 8fbd28a651..83cf101a1c 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -1079,3 +1079,31 @@ Use the high-level `PDClient` library for simplified integration, or use raw gRP For architecture details, see [Architecture Documentation](architecture.md). For configuration, see [Configuration Guide](configuration.md). + + +### Discovery identity pools + +Discovery labels remain arbitrary strings. A live `SERVICE_ID` pool is protected +when a registration supplies all three labels `GRAPHSPACE`, `SERVICE_NAME`, and +`SERVICE_ID`. Within one application, every live registration in that pool must +then carry the same complete tuple, including registrations with a different +version or address. Multiple replicas of the same tuple remain supported. +Registrations that mix incomplete or different tuples receive the existing +registration error header. An existing complete registration cannot drop its +identity labels through an update of the same address and version. + +A `SERVICE_ID` query fails with a readable gRPC error when a protected pool mixes +incomplete or different tuples. Generic pools with no complete identity tuple, +queries by raw labels, and discovery enumeration keep their existing behavior. +A generic-only pool must stop renewing and expire before a new complete identity +can claim that identifier. Protection ends when its live records expire; it is +not a permanent identifier reservation or proof of endpoint ownership. + +Upgrade PD before relying on this protection. Earlier Server releases already +send the complete tuple and can read the existing error header. Registration +remains asynchronous: a rejected Server may continue serving locally while its +heartbeat reports the conflict. Stop conflicting registrations and wait for +their TTL, or explicitly choose another service identity. No identity is renamed +automatically. Clients selecting a HugeGraph service should query its raw +`GRAPHSPACE` and `SERVICE_NAME`; a legacy identifier-only query into a +generic-only pool still has generic discovery semantics. diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/RegistryService.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/RegistryService.java index 43aedc645c..cb4aeca276 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/RegistryService.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/RegistryService.java @@ -17,28 +17,100 @@ package org.apache.hugegraph.pd; +import java.util.List; +import java.util.Objects; + import org.apache.hugegraph.pd.common.PDException; +import org.apache.hugegraph.pd.common.PDRuntimeException; import org.apache.hugegraph.pd.config.PDConfig; +import org.apache.hugegraph.pd.grpc.Pdpb; import org.apache.hugegraph.pd.grpc.discovery.NodeInfo; import org.apache.hugegraph.pd.grpc.discovery.NodeInfos; import org.apache.hugegraph.pd.grpc.discovery.Query; import org.apache.hugegraph.pd.meta.DiscoveryMetaStore; import org.apache.hugegraph.pd.meta.MetadataFactory; +import org.apache.hugegraph.pd.raft.RaftEngine; public class RegistryService { - private PDConfig pdConfig; - private DiscoveryMetaStore meta; + + // Shared across facades. Never acquired by the Raft apply thread. + private static final Object REGISTRATION_MONITOR = new Object(); + private static final String SERVICE_ID = "SERVICE_ID"; + private static final String GRAPHSPACE = "GRAPHSPACE"; + private static final String SERVICE_NAME = "SERVICE_NAME"; + private final PDConfig pdConfig; + private final DiscoveryMetaStore meta; public RegistryService(PDConfig config) { this.pdConfig = config; - meta = MetadataFactory.newDiscoveryMeta(config); + this.meta = MetadataFactory.newDiscoveryMeta(config); } - public void register(NodeInfo nodeInfo, int outTimes) throws PDException { - meta.register(nodeInfo, outTimes); + public void register(NodeInfo node, int outTimes) throws PDException { + synchronized (REGISTRATION_MONITOR) { + long term = readTerm(); + NodeInfo previous = this.meta.registration(node); + if (previous != null && complete(previous) && !complete(node)) { + throw conflict(); + } + if (node.containsLabels(SERVICE_ID)) { + List pool = pool(node.getAppName(), node.getLabelsOrThrow(SERVICE_ID)); + boolean protectedPool = complete(node) || pool.stream().anyMatch(RegistryService::complete); + if (protectedPool && (!complete(node) || + pool.stream().anyMatch(other -> !sameIdentity(node, other)))) { + throw conflict(); + } + } + this.meta.register(node, outTimes, term); + } } public NodeInfos getNodes(Query query) { - return meta.getNodes(query); + if (!query.containsLabels(SERVICE_ID)) { + return this.meta.getNodes(query); + } + synchronized (REGISTRATION_MONITOR) { + try { + readTerm(); + List pool = pool(query.getAppName(), query.getLabelsOrThrow(SERVICE_ID)); + for (NodeInfo node : pool) { + if (complete(node) && pool.stream().anyMatch(other -> + node.getAppName().equals(other.getAppName()) && !sameIdentity(node, other))) { + throw conflict(); + } + } + return this.meta.getNodesStrict(query); + } catch (PDException e) { + throw new PDRuntimeException(e.getErrorCode(), e.getMessage(), e); + } + } + } + + private long readTerm() throws PDException { + return this.pdConfig.getRaft().isEnable() ? + RaftEngine.getInstance().discoveryReadTerm() : -1; + } + + private List pool(String appName, String id) throws PDException { + return this.meta.getNodesStrict(Query.newBuilder().setAppName(appName) + .putLabels(SERVICE_ID, id).build()).getInfoList(); + } + + private static boolean complete(NodeInfo node) { + return node.containsLabels(SERVICE_ID) && node.containsLabels(GRAPHSPACE) && + node.containsLabels(SERVICE_NAME); + } + + private static boolean sameIdentity(NodeInfo first, NodeInfo second) { + return complete(second) && Objects.equals(first.getLabelsOrThrow(GRAPHSPACE), + second.getLabelsOrThrow(GRAPHSPACE)) && + Objects.equals(first.getLabelsOrThrow(SERVICE_NAME), second.getLabelsOrThrow(SERVICE_NAME)); + } + + private static PDException conflict() { + return new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, + "Discovery identity conflict: this SERVICE_ID pool contains an incomplete or different " + + "GRAPHSPACE/SERVICE_NAME tuple. Stop conflicting registrations and wait for their TTL, " + + "or explicitly select a different service identity."); } } diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/DiscoveryMetaStore.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/DiscoveryMetaStore.java index e774539c1e..6dd9c6dc82 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/DiscoveryMetaStore.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/DiscoveryMetaStore.java @@ -28,6 +28,7 @@ import org.apache.hugegraph.pd.grpc.discovery.NodeInfo; import org.apache.hugegraph.pd.grpc.discovery.NodeInfos; import org.apache.hugegraph.pd.grpc.discovery.Query; +import org.apache.hugegraph.pd.store.RaftKVStore; import lombok.extern.slf4j.Slf4j; @@ -50,6 +51,44 @@ public void register(NodeInfo nodeInfo, int outTimes) throws PDException { nodeInfo.toByteArray(), (nodeInfo.getInterval() / 1000) * outTimes); } + public NodeInfo registration(NodeInfo node) throws PDException { + byte[] value = getWithTTL(toKey(node.getAppName(), node.getVersion(), node.getAddress())); + try { + return value == null ? null : NodeInfo.parseFrom(value); + } catch (com.google.protobuf.InvalidProtocolBufferException e) { + throw new PDException(1, "Invalid discovery registration", e); + } + } + + public void register(NodeInfo node, int outTimes, long term) throws PDException { + if (term > 0) { + if (!(this.store instanceof RaftKVStore)) { + throw new PDException(1, "Discovery Raft store is unavailable"); + } + ((RaftKVStore) this.store).putDiscoveryWithTTL( + toKey(node.getAppName(), node.getVersion(), node.getAddress()), + node.toByteArray(), (node.getInterval() / 1000) * outTimes, term); + } else { + register(node, outTimes); + } + } + + public NodeInfos getNodesStrict(Query query) throws PDException { + List nodes = getInstanceListWithTTL( + NodeInfo.parser(), getPrefixBuilder(query.getAppName(), "").toString().getBytes()); + NodeInfos.Builder result = NodeInfos.newBuilder(); + for (NodeInfo node : nodes) { + if ((!query.getAppName().isEmpty() && !query.getAppName().equals(node.getAppName())) || + (!query.getVersion().isEmpty() && !query.getVersion().equals(node.getVersion()))) { + continue; + } + if (labelMatch(node, query)) { + result.addInfo(node); + } + } + return result.build(); + } + byte[] toKey(String appName, String version, String address) { StringBuilder builder = getPrefixBuilder(appName, version); builder.append(SPLITTER); diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java index 50eb1ca906..b744bc1870 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java @@ -47,6 +47,7 @@ import com.alipay.sofa.jraft.RaftGroupService; import com.alipay.sofa.jraft.Status; import com.alipay.sofa.jraft.conf.Configuration; +import com.alipay.sofa.jraft.closure.ReadIndexClosure; import com.alipay.sofa.jraft.core.Replicator; import com.alipay.sofa.jraft.core.State; import com.alipay.sofa.jraft.entity.PeerId; @@ -54,6 +55,7 @@ import com.alipay.sofa.jraft.error.RaftError; import com.alipay.sofa.jraft.option.NodeOptions; import com.alipay.sofa.jraft.option.RpcOptions; +import com.alipay.sofa.jraft.option.ReadOnlyOption; import com.alipay.sofa.jraft.rpc.RaftRpcServerFactory; import com.alipay.sofa.jraft.rpc.RpcServer; import com.alipay.sofa.jraft.rpc.impl.BoltRpcServer; @@ -397,6 +399,38 @@ void refreshAlivePeerCount() { /** * Add a raft task, and grpc sends data to raft through this interface */ + /** Wait for a linearizable read to be applied, and bind the following write to this term. */ + public long discoveryReadTerm() throws PDException { + Node node = this.raftNode; + long term = this.stateMachine.leaderTerm(); + if (node == null || term <= 0 || !isLeader()) { + throw new PDException(Pdpb.ErrorType.NOT_LEADER_VALUE, "Discovery leader is not ready"); + } + CompletableFuture applied = new CompletableFuture<>(); + node.readIndex(ReadOnlyOption.ReadOnlySafe, new byte[0], new ReadIndexClosure() { + @Override + public void run(Status status, long index, byte[] context) { + if (status.isOk()) { + applied.complete(null); + } else { + applied.completeExceptionally(new IllegalStateException(status.toString())); + } + } + }); + try { + applied.get(this.config.getRpcTimeout(), TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, "Discovery read interrupted"); + } catch (ExecutionException | TimeoutException e) { + throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, "Discovery read is unavailable"); + } + if (term != this.stateMachine.leaderTerm() || !isLeader()) { + throw new PDException(Pdpb.ErrorType.NOT_LEADER_VALUE, "Discovery leader changed"); + } + return term; + } + public void addTask(Task task) { if (!isLeader()) { KVStoreClosure closure = (KVStoreClosure) task.getDone(); diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java index cc9caaa900..7dbff41324 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java @@ -97,6 +97,10 @@ public void addStateListener(RaftStateListener listener) { stateListeners.add(listener); } + long leaderTerm() { + return this.leaderTerm.get(); + } + public boolean isLeader() { return this.leaderTerm.get() > 0; } diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/RaftKVStore.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/RaftKVStore.java index b61f07ac1d..260159e42f 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/RaftKVStore.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/RaftKVStore.java @@ -20,6 +20,7 @@ import java.nio.ByteBuffer; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.concurrent.TimeUnit; import org.apache.hugegraph.pd.common.PDException; @@ -130,6 +131,15 @@ public void putWithTTL(byte[] key, byte[] value, long ttl) throws PDException { } } + /** Keep the registration gate until apply is known, even if the RPC caller is interrupted. */ + public void putDiscoveryWithTTL(byte[] key, byte[] value, long ttl, long term) throws PDException { + try { + applyOperation(KVOperation.createPutWithTTL(key, value, ttl), term).join(); + } catch (CompletionException e) { + throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, "Discovery write failed", e); + } + } + @Override public void putWithTTL(byte[] key, byte[] value, long ttl, TimeUnit timeUnit) throws PDException { @@ -221,9 +231,14 @@ public void doLoadSnapshot(String snapshotPath) throws PDException { } private CompletableFuture applyOperation(final KVOperation op) throws PDException { + return applyOperation(op, -1); + } + + private CompletableFuture applyOperation(final KVOperation op, long term) throws PDException { CompletableFuture future = new CompletableFuture<>(); try { final Task task = new Task(); + task.setExpectedTerm(term); task.setData(ByteBuffer.wrap(op.toByteArray())); task.setDone(new RaftStateMachine.RaftClosureAdapter(op, new KVStoreClosure() { Object data; diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java index 00a652577f..4790ff7567 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java @@ -133,8 +133,13 @@ public void getNodes(Query request, io.grpc.stub.StreamObserver respo redirectToLeader(DiscoveryServiceGrpc.getGetNodesMethod(), request, responseObserver); return; } - responseObserver.onNext(register.getNodes(request)); - responseObserver.onCompleted(); + try { + responseObserver.onNext(register.getNodes(request)); + responseObserver.onCompleted(); + } catch (PDRuntimeException e) { + responseObserver.onError(io.grpc.Status.FAILED_PRECONDITION + .withDescription(e.getMessage()).asRuntimeException()); + } } public boolean isLeader() { diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDRestService.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDRestService.java index c9d55370cf..2aae9eb2dc 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDRestService.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDRestService.java @@ -210,7 +210,7 @@ public void onCompleted() { public ArrayList getNodeInfo(Query request) throws PDException { CountDownLatch latch = new CountDownLatch(1); final NodeInfos[] info = {null}; - RegistryRestResponse response = new RegistryRestResponse(); + final Throwable[] failure = {null}; ArrayList registryRestRequests = null; try { StreamObserver observer = new StreamObserver() { @@ -222,6 +222,7 @@ public void onNext(NodeInfos value) { @Override public void onError(Throwable t) { + failure[0] = t; latch.countDown(); } @@ -232,6 +233,12 @@ public void onCompleted() { }; this.discoveryService.getNodes(request, observer); latch.await(); + if (failure[0] != null) { + throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, failure[0].getMessage(), failure[0]); + } + if (info[0] == null) { + throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, "Discovery query completed without a reply"); + } List infoList = info[0].getInfoList(); registryRestRequests = new ArrayList(infoList.size()); for (int i = 0; i < infoList.size(); i++) { @@ -247,8 +254,8 @@ public void onCompleted() { registryRestRequests.add(registryRestRequest); } } catch (InterruptedException e) { - response.setErrorType(Pdpb.ErrorType.UNRECOGNIZED); - response.setMessage(e.getMessage()); + Thread.currentThread().interrupt(); + throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, "Discovery query interrupted", e); } return registryRestRequests; } diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java index 23ca938c51..ddfc3417c9 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java @@ -39,6 +39,7 @@ MetadataKeyHelperTest.class, HgKVStoreImplTest.class, PDNativeOwnerTest.class, + RegistryIdentityTest.class, PDConfigTest.class, ConfigServiceTest.class, IdServiceTest.class, diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RegistryIdentityTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RegistryIdentityTest.java new file mode 100644 index 0000000000..717152f06b --- /dev/null +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RegistryIdentityTest.java @@ -0,0 +1,166 @@ +/* + * 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. + */ + +package org.apache.hugegraph.pd.core; + +import java.nio.charset.StandardCharsets; +import java.util.UUID; + +import org.apache.hugegraph.pd.RegistryService; +import org.apache.hugegraph.pd.common.PDException; +import org.apache.hugegraph.pd.common.PDRuntimeException; +import org.apache.hugegraph.pd.config.PDConfig; +import org.apache.hugegraph.pd.grpc.discovery.NodeInfo; +import org.apache.hugegraph.pd.grpc.discovery.Query; +import org.apache.hugegraph.pd.meta.DiscoveryMetaStore; +import org.apache.hugegraph.pd.meta.MetadataFactory; +import org.apache.hugegraph.pd.store.HgKVStore; +import org.apache.hugegraph.pd.store.HgKVStoreImpl; +import org.apache.hugegraph.testutil.Whitebox; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +public class RegistryIdentityTest { + + @Rule + public TemporaryFolder temporary = new TemporaryFolder(); + private HgKVStore previous; + private HgKVStoreImpl store; + private PDConfig config; + private RegistryService registry; + private String app; + + @Before + public void open() throws Exception { + this.config = new PDConfig(); + this.config.setDataPath(this.temporary.newFolder().getPath()); + this.config.setRaft(this.config.new Raft()); + this.store = new HgKVStoreImpl(); + this.store.init(this.config); + this.previous = Whitebox.getInternalState(MetadataFactory.class, "store"); + Whitebox.setInternalState(MetadataFactory.class, "store", this.store); + this.registry = new RegistryService(this.config); + this.app = "identity-test-" + UUID.randomUUID(); + } + + @After + public void close() throws Exception { + try { + // Remove only this fixture's TTL records; the TTL cache is shared across stores. + DiscoveryMetaStore meta = new DiscoveryMetaStore(this.config); + for (NodeInfo node : meta.getNodes(Query.newBuilder().setAppName(this.app).build()) + .getInfoList()) { + remove(node); + } + } finally { + Whitebox.setInternalState(MetadataFactory.class, "store", this.previous); + this.store.close(); + } + } + + private NodeInfo node(String address, String name, boolean full) { + NodeInfo.Builder result = NodeInfo.newBuilder().setAppName(this.app).setVersion("v1") + .setAddress(address).setInterval(60000) + .putLabels("SERVICE_ID", "shared-id"); + if (full) { + result.putLabels("GRAPHSPACE", "team_a").putLabels("SERVICE_NAME", name); + } + return result.build(); + } + + private Query byId() { + return Query.newBuilder().setAppName(this.app).putLabels("SERVICE_ID", "shared-id").build(); + } + + private void remove(NodeInfo node) throws Exception { + this.store.removeWithTTL(("REGIS-" + node.getAppName() + "-" + node.getVersion() + "-" + + node.getAddress()).getBytes(StandardCharsets.UTF_8)); + } + + @Test + public void testCompleteTupleRejectsAliasAndPreservesReplicaVersions() throws Exception { + NodeInfo original = node("one", "Foo", true); + this.registry.register(original, 3); + Assert.assertThrows(PDException.class, () -> this.registry.register(node("two", "foo", true), 3)); + this.registry.register(original.toBuilder().setAddress("replica").setVersion("v2").build(), 3); + Assert.assertEquals(2, this.registry.getNodes(byId()).getInfoCount()); + } + + @Test + public void testGenericOnlyPoolKeepsArbitraryLabels() throws Exception { + this.registry.register(node("one", "", false), 3); + this.registry.register(node("two", "", false).toBuilder().putLabels("any", "other").build(), 3); + Assert.assertEquals(2, this.registry.getNodes(byId()).getInfoCount()); + } + + @Test + public void testSamePhysicalKeyCannotDropIdentityOrChangeAlias() throws Exception { + NodeInfo original = node("one", "Foo", true); + this.registry.register(original, 3); + Assert.assertThrows(PDException.class, () -> this.registry.register(node("one", "", false), 3)); + Assert.assertThrows(PDException.class, () -> this.registry.register( + original.toBuilder().clearLabels().build(), 3)); + Assert.assertThrows(PDException.class, () -> this.registry.register(node("one", "foo", true), 3)); + Assert.assertEquals("Foo", this.registry.getNodes(byId()).getInfo(0).getLabelsOrThrow("SERVICE_NAME")); + } + + @Test + public void testGenericMustExitBeforeFirstCompleteTuple() throws Exception { + NodeInfo generic = node("one", "", false); + this.registry.register(generic, 3); + Assert.assertThrows(PDException.class, () -> this.registry.register(node("one", "Foo", true), 3)); + Query raw = Query.newBuilder().setAppName(this.app).putLabels("GRAPHSPACE", "team_a") + .putLabels("SERVICE_NAME", "Foo").build(); + Assert.assertEquals(0, this.registry.getNodes(raw).getInfoCount()); + remove(generic); + this.registry.register(node("one", "Foo", true), 3); + Assert.assertEquals(1, this.registry.getNodes(raw).getInfoCount()); + } + + @Test + public void testDirtyPoolRejectsIdQueryAndRenewalButKeepsEnumeration() throws Exception { + // Seed actual legacy writes that predate the authority policy. + DiscoveryMetaStore meta = new DiscoveryMetaStore(this.config); + NodeInfo full = node("one", "Foo", true); + meta.register(full, 3); + meta.register(node("two", "", false).toBuilder().setVersion("v2").build(), 3); + Assert.assertThrows(PDRuntimeException.class, () -> this.registry.getNodes(byId())); + Assert.assertThrows(PDRuntimeException.class, () -> this.registry.getNodes( + byId().toBuilder().setVersion("v1").build())); + Assert.assertThrows(PDException.class, () -> this.registry.register(full, 3)); + Assert.assertEquals(2, this.registry.getNodes(Query.newBuilder().setAppName(this.app).build()).getInfoCount()); + } + + @Test + public void testExpiredGenericDoesNotReserveIdentity() throws Exception { + this.registry.register(node("one", "", false).toBuilder().setInterval(0).build(), 3); + this.registry.register(node("two", "Foo", true), 3); + Assert.assertEquals(1, this.registry.getNodes(byId()).getInfoCount()); + } + + @Test + public void testCompleteEmptyValuesAreComparedWithoutNewInputRules() throws Exception { + NodeInfo empty = node("one", "", true).toBuilder().putLabels("GRAPHSPACE", "").build(); + this.registry.register(empty, 3); + this.registry.register(empty.toBuilder().setAddress("two").build(), 3); + Assert.assertEquals(2, this.registry.getNodes(byId()).getInfoCount()); + } +} diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index bd66a0343f..b45899964e 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -1165,9 +1165,11 @@ private void registerServiceToPd(String graphSpace, Service service) { } else { LOG.warn("RegisterServiceToPd Error. {}", header.getError().getMessage()); + return; } } } + LOG.debug("PD service registration acknowledged"); if (!licenseValid) { LOG.warn("License is valid."); licenseValid = true; @@ -1185,7 +1187,7 @@ private void registerServiceToPd(String graphSpace, Service service) { String pdServiceId = register.registerService(config); service.pdServiceId(pdServiceId); - LOG.info("Success to register service to pd"); + LOG.info("Scheduled service registration with PD"); } catch (Exception e) { LOG.error("Failed to register service to pd", e); @@ -2173,7 +2175,7 @@ public Set getServiceUrls(String graphSpace, String service, PdRegisterType registerType) { Map configs = new HashMap<>(); if (StringUtils.isNotEmpty(graphSpace)) { - configs.put(PdRegisterLabel.REGISTER_TYPE.name(), graphSpace); + configs.put(PdRegisterLabel.GRAPHSPACE.name(), graphSpace); } if (StringUtils.isNotEmpty(service)) { configs.put(PdRegisterLabel.SERVICE_NAME.name(), service); From 2edfe1e90d58296ba925ef7c7d523668ec4f385e Mon Sep 17 00:00:00 2001 From: imbajin Date: Thu, 15 Oct 2026 01:35:14 +0800 Subject: [PATCH 08/11] fix(helm): require space-safe identity mapping - reject older image identity setters before config writes - preserve spaces through the native properties reader - cover missing and incomplete encoder capabilities - document pinned-image compatibility requirements --- helm/hugegraph/README.md | 4 ++ .../templates/server-deployment.yaml | 9 ++-- .../tests/server_discovery_test.yaml | 3 ++ helm/test_helm_package.py | 49 ++++++++++++++++++- 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/helm/hugegraph/README.md b/helm/hugegraph/README.md index 7acc49dce0..71ab6edb82 100644 --- a/helm/hugegraph/README.md +++ b/helm/hugegraph/README.md @@ -669,6 +669,10 @@ environment-to-property mappings before rewriting configuration or launching Server. An older or unrecognized entrypoint stops with a `compatible Server image` error instead of silently using `DEFAULT`. Select an image built with these mappings; the chart's default image tag is unchanged. +The service-name mapping must convert spaces to `\u0020` before writing +`server.service_id`. Bare mappings or encoders without this conversion are +refused because the Server's properties reader otherwise retains a backslash +before each space. Create the non-default graph space in the existing cluster before deploying Servers assigned to it; the chart diff --git a/helm/hugegraph/templates/server-deployment.yaml b/helm/hugegraph/templates/server-deployment.yaml index 3f2c5bad95..7b312ef24b 100644 --- a/helm/hugegraph/templates/server-deployment.yaml +++ b/helm/hugegraph/templates/server-deployment.yaml @@ -159,12 +159,11 @@ spec: fi {{- end }} # Older images ignore these optional identity variables. Require - # their actual env-to-property mappings before changing config. + # their actual mappings and service-name space encoding before changing config. if [[ -n "${HG_SERVER_SERVICE_ID:-}" ]]; then - if ! grep -Eq '^[[:space:]]*set_prop[[:space:]]+"server\.service_id"[[:space:]]+"\$\{HG_SERVER_SERVICE_ID\}"[[:space:]]+"\$\{REST_SERVER_CONF\}"([[:space:]]|$)' ./docker-entrypoint.sh && - ! (grep -Eq '^[[:space:]]*service_id_encoded=\$\(encode_prop_value[[:space:]]+"\$\{HG_SERVER_SERVICE_ID\}"' ./docker-entrypoint.sh && - grep -Eq '^[[:space:]]*set_prop_encoded[[:space:]]+"server\.service_id"[[:space:]]+"\$\{service_id_encoded\}"[[:space:]]+"\$\{REST_SERVER_CONF\}"([[:space:]]|$)' ./docker-entrypoint.sh); then - echo "non-default server.serviceId requires a compatible Server image with the HG_SERVER_SERVICE_ID mapping" >&2 + if ! grep -Eq '^[[:space:]]*service_id_encoded=\$\(encode_prop_value[[:space:]]+"\$\{HG_SERVER_SERVICE_ID\}"[[:space:]]*\|[[:space:]]*sed[[:space:]]+'"'"'s/\\\\ /\\\\u0020/g'"'"'\)[[:space:]]*$' ./docker-entrypoint.sh || + ! grep -Eq '^[[:space:]]*set_prop_encoded[[:space:]]+"server\.service_id"[[:space:]]+"\$\{service_id_encoded\}"[[:space:]]+"\$\{REST_SERVER_CONF\}"([[:space:]]|$)' ./docker-entrypoint.sh; then + echo "non-default server.serviceId requires a compatible Server image with a space-preserving HG_SERVER_SERVICE_ID mapping" >&2 exit 1 fi fi diff --git a/helm/hugegraph/tests/server_discovery_test.yaml b/helm/hugegraph/tests/server_discovery_test.yaml index cd9b9a3d1d..4d439d6113 100644 --- a/helm/hugegraph/tests/server_discovery_test.yaml +++ b/helm/hugegraph/tests/server_discovery_test.yaml @@ -99,6 +99,9 @@ tests: - equal: path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_SERVICE_ID")].value value: ops team + - matchRegex: + path: spec.template.spec.containers[0].args[0] + pattern: 'space-preserving HG_SERVER_SERVICE_ID mapping' - it: refuses an extraEnv that shadows the legacy service ID set: diff --git a/helm/test_helm_package.py b/helm/test_helm_package.py index 0e3896b0bb..502ba50427 100644 --- a/helm/test_helm_package.py +++ b/helm/test_helm_package.py @@ -116,6 +116,12 @@ def test_service_identity_requires_image_entrypoint_mappings(self): # Execute the actual image's validation and mappings, then stop before # storage initialization or Java launch. No properties encoder is copied. compatible = current.split("# ── Build wait-storage env", 1)[0] + "\nprintf launched > launched\n" + bare = compatible.replace( + 'set_prop_encoded "server.service_id" "${service_id_encoded}" "${REST_SERVER_CONF}"', + 'set_prop "server.service_id" "${HG_SERVER_SERVICE_ID}" "${REST_SERVER_CONF}"') + no_unicode = compatible.replace(r" | sed 's/\\ /\\u0020/g'", "") + self.assertNotEqual(compatible, bare) + self.assertNotEqual(compatible, no_unicode) comment_only = old + "\n".join("# " + line for line in current.splitlines()) + "\n" with tempfile.TemporaryDirectory(prefix="hugegraph-identity-wrapper-") as temporary: directory = Path(temporary) @@ -132,13 +138,46 @@ def test_service_identity_requires_image_entrypoint_mappings(self): entrypoint = directory / "docker-entrypoint.sh" launcher = directory / "wrapper.sh" launcher.write_text(script) + reader_classpath = os.environ.get("HG_CONFIG_TEST_CLASSPATH") + if reader_classpath: + (directory / "reader").mkdir() + subprocess.run(["javac", "-cp", reader_classpath, "-d", str(directory / "reader"), + str(ROOT / "hugegraph-server/hugegraph-dist/docker/test/ConfigReadback.java")], + check=True, capture_output=True, text=True, timeout=30) + reader_classpath = str(directory / "reader") + os.pathsep + reader_classpath + else: + print("SKIP: native service readback (HG_CONFIG_TEST_CLASSPATH is unset); " + "serialized encoder contract is checked") + # The real bare encoder serializes a space as backslash-space, + # which the existing HugeConfig contract preserves incorrectly. + for image in (bare, no_unicode): + config.write_text(original) + entrypoint.write_text(image) + direct = subprocess.run(["bash", str(entrypoint)], cwd=directory, + env={"PATH": executable_path, "HG_SERVER_SERVICE_ID": "ops team"}, + capture_output=True, text=True, timeout=10) + self.assertEqual(0, direct.returncode, direct.stderr) + self.assertIn(r"server.service_id=ops\ team" + "\n", config.read_text()) + if reader_classpath: + readback = subprocess.run(["java", "-cp", reader_classpath, "ConfigReadback", + str(config), "ops team"], capture_output=True, text=True, timeout=10) + self.assertNotEqual(0, readback.returncode) + self.assertIn("but loaded [ops\\ team]", readback.stderr) + print(next(line for line in readback.stderr.splitlines() if "but loaded" in line)) for name, image, identity, supported in ( ("old service", old, {"HG_SERVER_SERVICE_ID": "analytics"}, False), ("old graph space", old, {"HG_SERVER_GRAPHSPACE": "team_a"}, False), ("comment-only service", comment_only, {"HG_SERVER_SERVICE_ID": "analytics"}, False), ("comment-only graph space", comment_only, {"HG_SERVER_GRAPHSPACE": "team_a"}, False), + ("bare service mapping", bare, {"HG_SERVER_SERVICE_ID": "ops team"}, False), + ("encoded mapping without Unicode spaces", no_unicode, + {"HG_SERVER_SERVICE_ID": "ops team"}, False), ("current mappings", compatible, {"HG_SERVER_SERVICE_ID": "analytics", "HG_SERVER_GRAPHSPACE": "team_a"}, True), + ("current mapping with spaces", compatible, + {"HG_SERVER_SERVICE_ID": "ops team", "HG_SERVER_GRAPHSPACE": "team_a"}, True), + ("current mapping with boundary spaces", compatible, + {"HG_SERVER_SERVICE_ID": " ops team ", "HG_SERVER_GRAPHSPACE": "team_a"}, True), ("default old image", old, {}, True)): with self.subTest(case=name): config.write_text(original) @@ -155,7 +194,15 @@ def test_service_identity_requires_image_entrypoint_mappings(self): self.assertTrue((directory / "launched").exists()) self.assertIn("usePD=true\n", config.read_text()) if identity: - self.assertIn("server.service_id=analytics\n", config.read_text()) + service_id = identity["HG_SERVER_SERVICE_ID"] + encoded = service_id.replace(" ", r"\u0020") + self.assertIn("server.service_id=" + encoded + "\n", config.read_text()) + if reader_classpath: + readback = subprocess.run(["java", "-cp", reader_classpath, "ConfigReadback", + str(config), service_id], + capture_output=True, text=True, timeout=10) + self.assertEqual(0, readback.returncode, readback.stderr) + print(readback.stdout.strip()) self.assertIn("server.graphspace=team_a\n", config.read_text()) else: self.assertNotEqual(0, run.returncode, From 258685c8a20b0e95a14ce0858e33682513b629c9 Mon Sep 17 00:00:00 2001 From: imbajin Date: Thu, 15 Oct 2026 01:48:06 +0800 Subject: [PATCH 09/11] fix(server): cover discovery query labels - capture the real service URL query sent to PD - preserve graph space and raw service name filters - verify registration type and returned endpoints --- .../core/GraphManagerServiceNamesTest.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerServiceNamesTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerServiceNamesTest.java index fa02979daa..242e769724 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerServiceNamesTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerServiceNamesTest.java @@ -21,6 +21,7 @@ import java.lang.reflect.Field; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import org.apache.commons.configuration2.PropertiesConfiguration; @@ -31,15 +32,49 @@ import org.apache.hugegraph.meta.MetaManager; import org.apache.hugegraph.meta.lock.LockResult; import org.apache.hugegraph.meta.managers.ServiceMetaManager; +import org.apache.hugegraph.pd.client.DiscoveryClientImpl; +import org.apache.hugegraph.pd.grpc.discovery.NodeInfo; +import org.apache.hugegraph.pd.grpc.discovery.NodeInfos; +import org.apache.hugegraph.pd.grpc.discovery.Query; import org.apache.hugegraph.space.GraphSpace; import org.apache.hugegraph.space.Service; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.testutil.Whitebox; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.Mockito; public class GraphManagerServiceNamesTest { + @Test + public void testServiceUrlsPreservesRawIdentityAndRegisterType() throws Exception { + DiscoveryClientImpl client = Mockito.mock(DiscoveryClientImpl.class); + Mockito.when(client.getNodeInfos(Mockito.any(Query.class))) + .thenReturn(NodeInfos.newBuilder() + .addInfo(NodeInfo.newBuilder().setAddress("http://server:8080")) + .build()); + GraphManager manager = new GraphManager( + new HugeConfig(new PropertiesConfiguration()), + new EventHub("service-query-test")); + try { + setField(manager, "pdClient", client); + Class registerType = Class.forName(GraphManager.class.getName() + "$PdRegisterType"); + Object nodePort = Whitebox.getInternalState(registerType, "NODE_PORT"); + Set urls = Whitebox.invoke(GraphManager.class, + new Class[]{String.class, String.class, registerType}, + "getServiceUrls", manager, "team_a", "team/Analytics-prod", nodePort); + + ArgumentCaptor query = ArgumentCaptor.forClass(Query.class); + Mockito.verify(client).getNodeInfos(query.capture()); + Assert.assertEquals(Map.of("GRAPHSPACE", "team_a", + "SERVICE_NAME", "team/Analytics-prod", + "REGISTER_TYPE", "NODE_PORT"), query.getValue().getLabelsMap()); + Assert.assertEquals(Set.of("http://server:8080"), urls); + } finally { + manager.close(); + } + } + @Test public void testReloadPreservesCompleteServiceNames() { MetaDriver driver = storageDriver(); From 411835f49d744d3533a9b7312ef4504f88217fb4 Mon Sep 17 00:00:00 2001 From: imbajin Date: Thu, 15 Oct 2026 03:23:48 +0800 Subject: [PATCH 10/11] refactor: simplify discovery configuration - remove PD identity and Raft authority changes - preserve native configuration and raw service-name fixes - require matching feature-supporting chart and image builds - remove image source-shape detection and related tests --- docker/README.md | 12 +- helm/hugegraph/README.md | 30 ++-- .../templates/server-deployment.yaml | 14 -- .../tests/server_discovery_test.yaml | 9 - helm/test_helm_package.py | 119 ------------- hugegraph-pd/docs/api-reference.md | 28 --- .../apache/hugegraph/pd/RegistryService.java | 84 +-------- .../hugegraph/pd/meta/DiscoveryMetaStore.java | 39 ---- .../apache/hugegraph/pd/raft/RaftEngine.java | 34 ---- .../hugegraph/pd/raft/RaftStateMachine.java | 4 - .../hugegraph/pd/store/RaftKVStore.java | 15 -- .../pd/service/DiscoveryService.java | 9 +- .../hugegraph/pd/service/PDRestService.java | 13 +- .../hugegraph/pd/core/PDCoreSuiteTest.java | 1 - .../pd/core/RegistryIdentityTest.java | 166 ------------------ .../apache/hugegraph/core/GraphManager.java | 4 +- 16 files changed, 32 insertions(+), 549 deletions(-) delete mode 100644 hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RegistryIdentityTest.java diff --git a/docker/README.md b/docker/README.md index 82d802f2fd..2ba2bac15a 100644 --- a/docker/README.md +++ b/docker/README.md @@ -271,8 +271,8 @@ Standalone stores RocksDB data at `/hugegraph-server/rocksdb-data`. The HStore t ### Server service identity -Images built from current master accept `HG_SERVER_SERVICE_ID` and -`HG_SERVER_GRAPHSPACE`, which write `server.service_id` and `server.graphspace` +Server image builds containing this configuration mapping accept +`HG_SERVER_SERVICE_ID` and `HG_SERVER_GRAPHSPACE`, which write `server.service_id` and `server.graphspace` in `conf/rest-server.properties`. A property keeps its existing value when both its `HG_SERVER_*` variable and its older name are unset or empty; the shipped defaults are both `DEFAULT`. @@ -298,6 +298,14 @@ variables through from the host, so add explicit `environment:` entries to a local Compose override. The Helm chart exposes the same settings as [`server.serviceId` and `server.graphSpace`](../helm/hugegraph/README.md#server-service-identity). +Use a chart and Server image from the same source version containing these +mappings. Older release images without them do not apply these environment +values; select the matching image build explicitly. +PD's existing `SERVICE_ID` normalization is unchanged and can map different raw +names to the same identifier. Select `GRAPHSPACE` and the complete raw +`SERVICE_NAME` in discovery queries; identifier-only lookup does not provide a +uniqueness or isolation guarantee. + This selects the Server's PD service metadata. It does not change `server.path_graphspace` for REST paths that omit `/graphspaces/`, or configure Hubble's chosen service. Actual non-default service discovery, diff --git a/helm/hugegraph/README.md b/helm/hugegraph/README.md index 71ab6edb82..6949afaed9 100644 --- a/helm/hugegraph/README.md +++ b/helm/hugegraph/README.md @@ -646,17 +646,10 @@ this is a deployment input serialization limit, not a change to core service nam The full service ID, including `/` and `-`, is retained when service metadata is reloaded or its graph space is cleared. -PD protects a live discovery pool when its registrations provide the complete -`GRAPHSPACE`/`SERVICE_NAME`/`SERVICE_ID` tuple. Different raw names that map to the -same discovery identifier are rejected rather than merged into one endpoint -pool. The configured names and printable-character rules above are unchanged. -Upgrade PD first; old PD leaders do not provide this check. Registration is -asynchronous, so a conflicting Server can remain running locally while logging -a registration error. Stop conflicting heartbeats and wait for their TTL, or -explicitly select a different service identity. A generic-only pool must also -expire before a complete Server identity can join it. Clients should select the -raw graph space and service name; identifier-only queries to generic-only pools -keep their previous behavior. +PD's existing `SERVICE_ID` normalization is unchanged and can map different raw +names to the same identifier. Query with `GRAPHSPACE` and the complete raw +`SERVICE_NAME`; identifier-only discovery does not provide a uniqueness or +isolation guarantee. ```bash helm upgrade --install hg ./helm/hugegraph \ @@ -664,15 +657,12 @@ helm upgrade --install hg ./helm/hugegraph \ --set-string server.graphSpace=team_a ``` -For non-default identities, the startup wrapper checks the image entrypoint's -environment-to-property mappings before rewriting configuration or launching -Server. An older or unrecognized entrypoint stops with a `compatible Server -image` error instead of silently using `DEFAULT`. Select an image built with -these mappings; the chart's default image tag is unchanged. -The service-name mapping must convert spaces to `\u0020` before writing -`server.service_id`. Bare mappings or encoders without this conversion are -refused because the Server's properties reader otherwise retains a backslash -before each space. +These settings require a Server image build containing this configuration +mapping. Use a chart and image from the same source version with this feature; +older release images without the mappings do not apply these values. Select the +matching image tag or digest explicitly; the chart's default image tag is unchanged. +Service-name spaces are written as `\u0020` so the Server's properties reader +retains the configured value. Create the non-default graph space in the existing cluster before deploying Servers assigned to it; the chart diff --git a/helm/hugegraph/templates/server-deployment.yaml b/helm/hugegraph/templates/server-deployment.yaml index 7b312ef24b..db66db59d4 100644 --- a/helm/hugegraph/templates/server-deployment.yaml +++ b/helm/hugegraph/templates/server-deployment.yaml @@ -158,20 +158,6 @@ spec: exit 1 fi {{- end }} - # Older images ignore these optional identity variables. Require - # their actual mappings and service-name space encoding before changing config. - if [[ -n "${HG_SERVER_SERVICE_ID:-}" ]]; then - if ! grep -Eq '^[[:space:]]*service_id_encoded=\$\(encode_prop_value[[:space:]]+"\$\{HG_SERVER_SERVICE_ID\}"[[:space:]]*\|[[:space:]]*sed[[:space:]]+'"'"'s/\\\\ /\\\\u0020/g'"'"'\)[[:space:]]*$' ./docker-entrypoint.sh || - ! grep -Eq '^[[:space:]]*set_prop_encoded[[:space:]]+"server\.service_id"[[:space:]]+"\$\{service_id_encoded\}"[[:space:]]+"\$\{REST_SERVER_CONF\}"([[:space:]]|$)' ./docker-entrypoint.sh; then - echo "non-default server.serviceId requires a compatible Server image with a space-preserving HG_SERVER_SERVICE_ID mapping" >&2 - exit 1 - fi - fi - if [[ -n "${HG_SERVER_GRAPHSPACE:-}" ]] && - ! grep -Eq '^[[:space:]]*set_prop[[:space:]]+"server\.graphspace"[[:space:]]+"\$\{HG_SERVER_GRAPHSPACE\}"[[:space:]]+"\$\{REST_SERVER_CONF\}"([[:space:]]|$)' ./docker-entrypoint.sh; then - echo "non-default server.graphSpace requires a compatible Server image with the HG_SERVER_GRAPHSPACE mapping" >&2 - exit 1 - fi CONF=./conf/rest-server.properties if [[ ! -r "${CONF}" ]]; then echo "missing or unreadable ${CONF}; refusing to write a stub config" >&2 diff --git a/helm/hugegraph/tests/server_discovery_test.yaml b/helm/hugegraph/tests/server_discovery_test.yaml index 4d439d6113..92d5adfaaa 100644 --- a/helm/hugegraph/tests/server_discovery_test.yaml +++ b/helm/hugegraph/tests/server_discovery_test.yaml @@ -66,12 +66,6 @@ tests: - equal: path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_GRAPHSPACE")].value value: team_a - - matchRegex: - path: spec.template.spec.containers[0].args[0] - pattern: 'non-default server\.serviceId requires a compatible Server image' - - matchRegex: - path: spec.template.spec.containers[0].args[0] - pattern: 'non-default server\.graphSpace requires a compatible Server image' - it: refuses an extraEnv that shadows the service ID set: @@ -99,9 +93,6 @@ tests: - equal: path: spec.template.spec.containers[0].env[?(@.name=="HG_SERVER_SERVICE_ID")].value value: ops team - - matchRegex: - path: spec.template.spec.containers[0].args[0] - pattern: 'space-preserving HG_SERVER_SERVICE_ID mapping' - it: refuses an extraEnv that shadows the legacy service ID set: diff --git a/helm/test_helm_package.py b/helm/test_helm_package.py index 502ba50427..ad43d5d399 100644 --- a/helm/test_helm_package.py +++ b/helm/test_helm_package.py @@ -17,7 +17,6 @@ """Validate the artifact shipped to Helm, beyond source template tests.""" from pathlib import Path -import os import shutil import subprocess import tarfile @@ -95,124 +94,6 @@ def test_packaged_chart_renders_each_preset(self): self.assertTrue(b"kind: Deployment" in rendered.stdout, "Packaged chart did not render a Deployment") - def test_service_identity_requires_image_entrypoint_mappings(self): - command = ["helm", "template", "identity-check", str(self.package), - "--show-only", "templates/server-deployment.yaml", - "--set", "server.auth.enabled=false"] - rendered = subprocess.run(command, check=True, capture_output=True, text=True).stdout - # Read the literal shell block without adding a YAML dependency. - lines = rendered.splitlines() - start = lines.index(" args:") + 2 - shell = [] - for line in lines[start:]: - if line.strip() and not line.startswith(" "): - break - shell.append(line[14:]) - script = "\n".join(shell) + "\n" - self.assertTrue(script.startswith("set -euo pipefail\n")) - original = "server.service_id=DEFAULT\nserver.graphspace=DEFAULT\nusePD=false\n" - old = "#!/bin/bash\nprintf launched > launched\n" - current = (ROOT / "hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh").read_text() - # Execute the actual image's validation and mappings, then stop before - # storage initialization or Java launch. No properties encoder is copied. - compatible = current.split("# ── Build wait-storage env", 1)[0] + "\nprintf launched > launched\n" - bare = compatible.replace( - 'set_prop_encoded "server.service_id" "${service_id_encoded}" "${REST_SERVER_CONF}"', - 'set_prop "server.service_id" "${HG_SERVER_SERVICE_ID}" "${REST_SERVER_CONF}"') - no_unicode = compatible.replace(r" | sed 's/\\ /\\u0020/g'", "") - self.assertNotEqual(compatible, bare) - self.assertNotEqual(compatible, no_unicode) - comment_only = old + "\n".join("# " + line for line in current.splitlines()) + "\n" - with tempfile.TemporaryDirectory(prefix="hugegraph-identity-wrapper-") as temporary: - directory = Path(temporary) - (directory / "conf/graphs").mkdir(parents=True) - (directory / "conf/graphs/hugegraph.properties").write_text("backend=hstore\n") - # The image mapping uses GNU sed. Reuse it when running on macOS; - # Linux CI already provides it as sed. - executable_path = os.environ["PATH"] - if shutil.which("gsed"): - (directory / "bin").mkdir() - (directory / "bin/sed").symlink_to(shutil.which("gsed")) - executable_path = str(directory / "bin") + os.pathsep + executable_path - config = directory / "conf/rest-server.properties" - entrypoint = directory / "docker-entrypoint.sh" - launcher = directory / "wrapper.sh" - launcher.write_text(script) - reader_classpath = os.environ.get("HG_CONFIG_TEST_CLASSPATH") - if reader_classpath: - (directory / "reader").mkdir() - subprocess.run(["javac", "-cp", reader_classpath, "-d", str(directory / "reader"), - str(ROOT / "hugegraph-server/hugegraph-dist/docker/test/ConfigReadback.java")], - check=True, capture_output=True, text=True, timeout=30) - reader_classpath = str(directory / "reader") + os.pathsep + reader_classpath - else: - print("SKIP: native service readback (HG_CONFIG_TEST_CLASSPATH is unset); " - "serialized encoder contract is checked") - # The real bare encoder serializes a space as backslash-space, - # which the existing HugeConfig contract preserves incorrectly. - for image in (bare, no_unicode): - config.write_text(original) - entrypoint.write_text(image) - direct = subprocess.run(["bash", str(entrypoint)], cwd=directory, - env={"PATH": executable_path, "HG_SERVER_SERVICE_ID": "ops team"}, - capture_output=True, text=True, timeout=10) - self.assertEqual(0, direct.returncode, direct.stderr) - self.assertIn(r"server.service_id=ops\ team" + "\n", config.read_text()) - if reader_classpath: - readback = subprocess.run(["java", "-cp", reader_classpath, "ConfigReadback", - str(config), "ops team"], capture_output=True, text=True, timeout=10) - self.assertNotEqual(0, readback.returncode) - self.assertIn("but loaded [ops\\ team]", readback.stderr) - print(next(line for line in readback.stderr.splitlines() if "but loaded" in line)) - for name, image, identity, supported in ( - ("old service", old, {"HG_SERVER_SERVICE_ID": "analytics"}, False), - ("old graph space", old, {"HG_SERVER_GRAPHSPACE": "team_a"}, False), - ("comment-only service", comment_only, {"HG_SERVER_SERVICE_ID": "analytics"}, False), - ("comment-only graph space", comment_only, {"HG_SERVER_GRAPHSPACE": "team_a"}, False), - ("bare service mapping", bare, {"HG_SERVER_SERVICE_ID": "ops team"}, False), - ("encoded mapping without Unicode spaces", no_unicode, - {"HG_SERVER_SERVICE_ID": "ops team"}, False), - ("current mappings", compatible, - {"HG_SERVER_SERVICE_ID": "analytics", "HG_SERVER_GRAPHSPACE": "team_a"}, True), - ("current mapping with spaces", compatible, - {"HG_SERVER_SERVICE_ID": "ops team", "HG_SERVER_GRAPHSPACE": "team_a"}, True), - ("current mapping with boundary spaces", compatible, - {"HG_SERVER_SERVICE_ID": " ops team ", "HG_SERVER_GRAPHSPACE": "team_a"}, True), - ("default old image", old, {}, True)): - with self.subTest(case=name): - config.write_text(original) - entrypoint.write_text(image) - entrypoint.chmod(0o700) - (directory / "launched").unlink(missing_ok=True) - environment = {"PATH": executable_path, - "HG_SERVER_PD_PEERS": "fixture:8686", - "HG_SERVER_URLS_TO_PD": "http://fixture:8080", **identity} - run = subprocess.run(["bash", str(launcher)], cwd=directory, env=environment, - capture_output=True, text=True, timeout=10) - if supported: - self.assertEqual(0, run.returncode, run.stderr) - self.assertTrue((directory / "launched").exists()) - self.assertIn("usePD=true\n", config.read_text()) - if identity: - service_id = identity["HG_SERVER_SERVICE_ID"] - encoded = service_id.replace(" ", r"\u0020") - self.assertIn("server.service_id=" + encoded + "\n", config.read_text()) - if reader_classpath: - readback = subprocess.run(["java", "-cp", reader_classpath, "ConfigReadback", - str(config), service_id], - capture_output=True, text=True, timeout=10) - self.assertEqual(0, readback.returncode, readback.stderr) - print(readback.stdout.strip()) - self.assertIn("server.graphspace=team_a\n", config.read_text()) - else: - self.assertNotEqual(0, run.returncode, - "Unsupported image launched=" + - str((directory / "launched").exists()) + - "; config changed=" + str(config.read_text() != original)) - self.assertIn("compatible Server image", run.stderr) - self.assertEqual(original, config.read_text(), "Rejected image changed config") - self.assertFalse((directory / "launched").exists(), "Rejected image was launched") - if __name__ == "__main__": unittest.main() diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 83cf101a1c..8fbd28a651 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -1079,31 +1079,3 @@ Use the high-level `PDClient` library for simplified integration, or use raw gRP For architecture details, see [Architecture Documentation](architecture.md). For configuration, see [Configuration Guide](configuration.md). - - -### Discovery identity pools - -Discovery labels remain arbitrary strings. A live `SERVICE_ID` pool is protected -when a registration supplies all three labels `GRAPHSPACE`, `SERVICE_NAME`, and -`SERVICE_ID`. Within one application, every live registration in that pool must -then carry the same complete tuple, including registrations with a different -version or address. Multiple replicas of the same tuple remain supported. -Registrations that mix incomplete or different tuples receive the existing -registration error header. An existing complete registration cannot drop its -identity labels through an update of the same address and version. - -A `SERVICE_ID` query fails with a readable gRPC error when a protected pool mixes -incomplete or different tuples. Generic pools with no complete identity tuple, -queries by raw labels, and discovery enumeration keep their existing behavior. -A generic-only pool must stop renewing and expire before a new complete identity -can claim that identifier. Protection ends when its live records expire; it is -not a permanent identifier reservation or proof of endpoint ownership. - -Upgrade PD before relying on this protection. Earlier Server releases already -send the complete tuple and can read the existing error header. Registration -remains asynchronous: a rejected Server may continue serving locally while its -heartbeat reports the conflict. Stop conflicting registrations and wait for -their TTL, or explicitly choose another service identity. No identity is renamed -automatically. Clients selecting a HugeGraph service should query its raw -`GRAPHSPACE` and `SERVICE_NAME`; a legacy identifier-only query into a -generic-only pool still has generic discovery semantics. diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/RegistryService.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/RegistryService.java index cb4aeca276..43aedc645c 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/RegistryService.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/RegistryService.java @@ -17,100 +17,28 @@ package org.apache.hugegraph.pd; -import java.util.List; -import java.util.Objects; - import org.apache.hugegraph.pd.common.PDException; -import org.apache.hugegraph.pd.common.PDRuntimeException; import org.apache.hugegraph.pd.config.PDConfig; -import org.apache.hugegraph.pd.grpc.Pdpb; import org.apache.hugegraph.pd.grpc.discovery.NodeInfo; import org.apache.hugegraph.pd.grpc.discovery.NodeInfos; import org.apache.hugegraph.pd.grpc.discovery.Query; import org.apache.hugegraph.pd.meta.DiscoveryMetaStore; import org.apache.hugegraph.pd.meta.MetadataFactory; -import org.apache.hugegraph.pd.raft.RaftEngine; public class RegistryService { - - // Shared across facades. Never acquired by the Raft apply thread. - private static final Object REGISTRATION_MONITOR = new Object(); - private static final String SERVICE_ID = "SERVICE_ID"; - private static final String GRAPHSPACE = "GRAPHSPACE"; - private static final String SERVICE_NAME = "SERVICE_NAME"; - private final PDConfig pdConfig; - private final DiscoveryMetaStore meta; + private PDConfig pdConfig; + private DiscoveryMetaStore meta; public RegistryService(PDConfig config) { this.pdConfig = config; - this.meta = MetadataFactory.newDiscoveryMeta(config); + meta = MetadataFactory.newDiscoveryMeta(config); } - public void register(NodeInfo node, int outTimes) throws PDException { - synchronized (REGISTRATION_MONITOR) { - long term = readTerm(); - NodeInfo previous = this.meta.registration(node); - if (previous != null && complete(previous) && !complete(node)) { - throw conflict(); - } - if (node.containsLabels(SERVICE_ID)) { - List pool = pool(node.getAppName(), node.getLabelsOrThrow(SERVICE_ID)); - boolean protectedPool = complete(node) || pool.stream().anyMatch(RegistryService::complete); - if (protectedPool && (!complete(node) || - pool.stream().anyMatch(other -> !sameIdentity(node, other)))) { - throw conflict(); - } - } - this.meta.register(node, outTimes, term); - } + public void register(NodeInfo nodeInfo, int outTimes) throws PDException { + meta.register(nodeInfo, outTimes); } public NodeInfos getNodes(Query query) { - if (!query.containsLabels(SERVICE_ID)) { - return this.meta.getNodes(query); - } - synchronized (REGISTRATION_MONITOR) { - try { - readTerm(); - List pool = pool(query.getAppName(), query.getLabelsOrThrow(SERVICE_ID)); - for (NodeInfo node : pool) { - if (complete(node) && pool.stream().anyMatch(other -> - node.getAppName().equals(other.getAppName()) && !sameIdentity(node, other))) { - throw conflict(); - } - } - return this.meta.getNodesStrict(query); - } catch (PDException e) { - throw new PDRuntimeException(e.getErrorCode(), e.getMessage(), e); - } - } - } - - private long readTerm() throws PDException { - return this.pdConfig.getRaft().isEnable() ? - RaftEngine.getInstance().discoveryReadTerm() : -1; - } - - private List pool(String appName, String id) throws PDException { - return this.meta.getNodesStrict(Query.newBuilder().setAppName(appName) - .putLabels(SERVICE_ID, id).build()).getInfoList(); - } - - private static boolean complete(NodeInfo node) { - return node.containsLabels(SERVICE_ID) && node.containsLabels(GRAPHSPACE) && - node.containsLabels(SERVICE_NAME); - } - - private static boolean sameIdentity(NodeInfo first, NodeInfo second) { - return complete(second) && Objects.equals(first.getLabelsOrThrow(GRAPHSPACE), - second.getLabelsOrThrow(GRAPHSPACE)) && - Objects.equals(first.getLabelsOrThrow(SERVICE_NAME), second.getLabelsOrThrow(SERVICE_NAME)); - } - - private static PDException conflict() { - return new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, - "Discovery identity conflict: this SERVICE_ID pool contains an incomplete or different " + - "GRAPHSPACE/SERVICE_NAME tuple. Stop conflicting registrations and wait for their TTL, " + - "or explicitly select a different service identity."); + return meta.getNodes(query); } } diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/DiscoveryMetaStore.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/DiscoveryMetaStore.java index 6dd9c6dc82..e774539c1e 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/DiscoveryMetaStore.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/DiscoveryMetaStore.java @@ -28,7 +28,6 @@ import org.apache.hugegraph.pd.grpc.discovery.NodeInfo; import org.apache.hugegraph.pd.grpc.discovery.NodeInfos; import org.apache.hugegraph.pd.grpc.discovery.Query; -import org.apache.hugegraph.pd.store.RaftKVStore; import lombok.extern.slf4j.Slf4j; @@ -51,44 +50,6 @@ public void register(NodeInfo nodeInfo, int outTimes) throws PDException { nodeInfo.toByteArray(), (nodeInfo.getInterval() / 1000) * outTimes); } - public NodeInfo registration(NodeInfo node) throws PDException { - byte[] value = getWithTTL(toKey(node.getAppName(), node.getVersion(), node.getAddress())); - try { - return value == null ? null : NodeInfo.parseFrom(value); - } catch (com.google.protobuf.InvalidProtocolBufferException e) { - throw new PDException(1, "Invalid discovery registration", e); - } - } - - public void register(NodeInfo node, int outTimes, long term) throws PDException { - if (term > 0) { - if (!(this.store instanceof RaftKVStore)) { - throw new PDException(1, "Discovery Raft store is unavailable"); - } - ((RaftKVStore) this.store).putDiscoveryWithTTL( - toKey(node.getAppName(), node.getVersion(), node.getAddress()), - node.toByteArray(), (node.getInterval() / 1000) * outTimes, term); - } else { - register(node, outTimes); - } - } - - public NodeInfos getNodesStrict(Query query) throws PDException { - List nodes = getInstanceListWithTTL( - NodeInfo.parser(), getPrefixBuilder(query.getAppName(), "").toString().getBytes()); - NodeInfos.Builder result = NodeInfos.newBuilder(); - for (NodeInfo node : nodes) { - if ((!query.getAppName().isEmpty() && !query.getAppName().equals(node.getAppName())) || - (!query.getVersion().isEmpty() && !query.getVersion().equals(node.getVersion()))) { - continue; - } - if (labelMatch(node, query)) { - result.addInfo(node); - } - } - return result.build(); - } - byte[] toKey(String appName, String version, String address) { StringBuilder builder = getPrefixBuilder(appName, version); builder.append(SPLITTER); diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java index b744bc1870..50eb1ca906 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java @@ -47,7 +47,6 @@ import com.alipay.sofa.jraft.RaftGroupService; import com.alipay.sofa.jraft.Status; import com.alipay.sofa.jraft.conf.Configuration; -import com.alipay.sofa.jraft.closure.ReadIndexClosure; import com.alipay.sofa.jraft.core.Replicator; import com.alipay.sofa.jraft.core.State; import com.alipay.sofa.jraft.entity.PeerId; @@ -55,7 +54,6 @@ import com.alipay.sofa.jraft.error.RaftError; import com.alipay.sofa.jraft.option.NodeOptions; import com.alipay.sofa.jraft.option.RpcOptions; -import com.alipay.sofa.jraft.option.ReadOnlyOption; import com.alipay.sofa.jraft.rpc.RaftRpcServerFactory; import com.alipay.sofa.jraft.rpc.RpcServer; import com.alipay.sofa.jraft.rpc.impl.BoltRpcServer; @@ -399,38 +397,6 @@ void refreshAlivePeerCount() { /** * Add a raft task, and grpc sends data to raft through this interface */ - /** Wait for a linearizable read to be applied, and bind the following write to this term. */ - public long discoveryReadTerm() throws PDException { - Node node = this.raftNode; - long term = this.stateMachine.leaderTerm(); - if (node == null || term <= 0 || !isLeader()) { - throw new PDException(Pdpb.ErrorType.NOT_LEADER_VALUE, "Discovery leader is not ready"); - } - CompletableFuture applied = new CompletableFuture<>(); - node.readIndex(ReadOnlyOption.ReadOnlySafe, new byte[0], new ReadIndexClosure() { - @Override - public void run(Status status, long index, byte[] context) { - if (status.isOk()) { - applied.complete(null); - } else { - applied.completeExceptionally(new IllegalStateException(status.toString())); - } - } - }); - try { - applied.get(this.config.getRpcTimeout(), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, "Discovery read interrupted"); - } catch (ExecutionException | TimeoutException e) { - throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, "Discovery read is unavailable"); - } - if (term != this.stateMachine.leaderTerm() || !isLeader()) { - throw new PDException(Pdpb.ErrorType.NOT_LEADER_VALUE, "Discovery leader changed"); - } - return term; - } - public void addTask(Task task) { if (!isLeader()) { KVStoreClosure closure = (KVStoreClosure) task.getDone(); diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java index 7dbff41324..cc9caaa900 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java @@ -97,10 +97,6 @@ public void addStateListener(RaftStateListener listener) { stateListeners.add(listener); } - long leaderTerm() { - return this.leaderTerm.get(); - } - public boolean isLeader() { return this.leaderTerm.get() > 0; } diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/RaftKVStore.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/RaftKVStore.java index 260159e42f..b61f07ac1d 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/RaftKVStore.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/RaftKVStore.java @@ -20,7 +20,6 @@ import java.nio.ByteBuffer; import java.util.List; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; import java.util.concurrent.TimeUnit; import org.apache.hugegraph.pd.common.PDException; @@ -131,15 +130,6 @@ public void putWithTTL(byte[] key, byte[] value, long ttl) throws PDException { } } - /** Keep the registration gate until apply is known, even if the RPC caller is interrupted. */ - public void putDiscoveryWithTTL(byte[] key, byte[] value, long ttl, long term) throws PDException { - try { - applyOperation(KVOperation.createPutWithTTL(key, value, ttl), term).join(); - } catch (CompletionException e) { - throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, "Discovery write failed", e); - } - } - @Override public void putWithTTL(byte[] key, byte[] value, long ttl, TimeUnit timeUnit) throws PDException { @@ -231,14 +221,9 @@ public void doLoadSnapshot(String snapshotPath) throws PDException { } private CompletableFuture applyOperation(final KVOperation op) throws PDException { - return applyOperation(op, -1); - } - - private CompletableFuture applyOperation(final KVOperation op, long term) throws PDException { CompletableFuture future = new CompletableFuture<>(); try { final Task task = new Task(); - task.setExpectedTerm(term); task.setData(ByteBuffer.wrap(op.toByteArray())); task.setDone(new RaftStateMachine.RaftClosureAdapter(op, new KVStoreClosure() { Object data; diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java index 4790ff7567..00a652577f 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java @@ -133,13 +133,8 @@ public void getNodes(Query request, io.grpc.stub.StreamObserver respo redirectToLeader(DiscoveryServiceGrpc.getGetNodesMethod(), request, responseObserver); return; } - try { - responseObserver.onNext(register.getNodes(request)); - responseObserver.onCompleted(); - } catch (PDRuntimeException e) { - responseObserver.onError(io.grpc.Status.FAILED_PRECONDITION - .withDescription(e.getMessage()).asRuntimeException()); - } + responseObserver.onNext(register.getNodes(request)); + responseObserver.onCompleted(); } public boolean isLeader() { diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDRestService.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDRestService.java index 2aae9eb2dc..c9d55370cf 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDRestService.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDRestService.java @@ -210,7 +210,7 @@ public void onCompleted() { public ArrayList getNodeInfo(Query request) throws PDException { CountDownLatch latch = new CountDownLatch(1); final NodeInfos[] info = {null}; - final Throwable[] failure = {null}; + RegistryRestResponse response = new RegistryRestResponse(); ArrayList registryRestRequests = null; try { StreamObserver observer = new StreamObserver() { @@ -222,7 +222,6 @@ public void onNext(NodeInfos value) { @Override public void onError(Throwable t) { - failure[0] = t; latch.countDown(); } @@ -233,12 +232,6 @@ public void onCompleted() { }; this.discoveryService.getNodes(request, observer); latch.await(); - if (failure[0] != null) { - throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, failure[0].getMessage(), failure[0]); - } - if (info[0] == null) { - throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, "Discovery query completed without a reply"); - } List infoList = info[0].getInfoList(); registryRestRequests = new ArrayList(infoList.size()); for (int i = 0; i < infoList.size(); i++) { @@ -254,8 +247,8 @@ public void onCompleted() { registryRestRequests.add(registryRestRequest); } } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new PDException(Pdpb.ErrorType.UNKNOWN_VALUE, "Discovery query interrupted", e); + response.setErrorType(Pdpb.ErrorType.UNRECOGNIZED); + response.setMessage(e.getMessage()); } return registryRestRequests; } diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java index ddfc3417c9..23ca938c51 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java @@ -39,7 +39,6 @@ MetadataKeyHelperTest.class, HgKVStoreImplTest.class, PDNativeOwnerTest.class, - RegistryIdentityTest.class, PDConfigTest.class, ConfigServiceTest.class, IdServiceTest.class, diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RegistryIdentityTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RegistryIdentityTest.java deleted file mode 100644 index 717152f06b..0000000000 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RegistryIdentityTest.java +++ /dev/null @@ -1,166 +0,0 @@ -/* - * 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. - */ - -package org.apache.hugegraph.pd.core; - -import java.nio.charset.StandardCharsets; -import java.util.UUID; - -import org.apache.hugegraph.pd.RegistryService; -import org.apache.hugegraph.pd.common.PDException; -import org.apache.hugegraph.pd.common.PDRuntimeException; -import org.apache.hugegraph.pd.config.PDConfig; -import org.apache.hugegraph.pd.grpc.discovery.NodeInfo; -import org.apache.hugegraph.pd.grpc.discovery.Query; -import org.apache.hugegraph.pd.meta.DiscoveryMetaStore; -import org.apache.hugegraph.pd.meta.MetadataFactory; -import org.apache.hugegraph.pd.store.HgKVStore; -import org.apache.hugegraph.pd.store.HgKVStoreImpl; -import org.apache.hugegraph.testutil.Whitebox; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; - -public class RegistryIdentityTest { - - @Rule - public TemporaryFolder temporary = new TemporaryFolder(); - private HgKVStore previous; - private HgKVStoreImpl store; - private PDConfig config; - private RegistryService registry; - private String app; - - @Before - public void open() throws Exception { - this.config = new PDConfig(); - this.config.setDataPath(this.temporary.newFolder().getPath()); - this.config.setRaft(this.config.new Raft()); - this.store = new HgKVStoreImpl(); - this.store.init(this.config); - this.previous = Whitebox.getInternalState(MetadataFactory.class, "store"); - Whitebox.setInternalState(MetadataFactory.class, "store", this.store); - this.registry = new RegistryService(this.config); - this.app = "identity-test-" + UUID.randomUUID(); - } - - @After - public void close() throws Exception { - try { - // Remove only this fixture's TTL records; the TTL cache is shared across stores. - DiscoveryMetaStore meta = new DiscoveryMetaStore(this.config); - for (NodeInfo node : meta.getNodes(Query.newBuilder().setAppName(this.app).build()) - .getInfoList()) { - remove(node); - } - } finally { - Whitebox.setInternalState(MetadataFactory.class, "store", this.previous); - this.store.close(); - } - } - - private NodeInfo node(String address, String name, boolean full) { - NodeInfo.Builder result = NodeInfo.newBuilder().setAppName(this.app).setVersion("v1") - .setAddress(address).setInterval(60000) - .putLabels("SERVICE_ID", "shared-id"); - if (full) { - result.putLabels("GRAPHSPACE", "team_a").putLabels("SERVICE_NAME", name); - } - return result.build(); - } - - private Query byId() { - return Query.newBuilder().setAppName(this.app).putLabels("SERVICE_ID", "shared-id").build(); - } - - private void remove(NodeInfo node) throws Exception { - this.store.removeWithTTL(("REGIS-" + node.getAppName() + "-" + node.getVersion() + "-" + - node.getAddress()).getBytes(StandardCharsets.UTF_8)); - } - - @Test - public void testCompleteTupleRejectsAliasAndPreservesReplicaVersions() throws Exception { - NodeInfo original = node("one", "Foo", true); - this.registry.register(original, 3); - Assert.assertThrows(PDException.class, () -> this.registry.register(node("two", "foo", true), 3)); - this.registry.register(original.toBuilder().setAddress("replica").setVersion("v2").build(), 3); - Assert.assertEquals(2, this.registry.getNodes(byId()).getInfoCount()); - } - - @Test - public void testGenericOnlyPoolKeepsArbitraryLabels() throws Exception { - this.registry.register(node("one", "", false), 3); - this.registry.register(node("two", "", false).toBuilder().putLabels("any", "other").build(), 3); - Assert.assertEquals(2, this.registry.getNodes(byId()).getInfoCount()); - } - - @Test - public void testSamePhysicalKeyCannotDropIdentityOrChangeAlias() throws Exception { - NodeInfo original = node("one", "Foo", true); - this.registry.register(original, 3); - Assert.assertThrows(PDException.class, () -> this.registry.register(node("one", "", false), 3)); - Assert.assertThrows(PDException.class, () -> this.registry.register( - original.toBuilder().clearLabels().build(), 3)); - Assert.assertThrows(PDException.class, () -> this.registry.register(node("one", "foo", true), 3)); - Assert.assertEquals("Foo", this.registry.getNodes(byId()).getInfo(0).getLabelsOrThrow("SERVICE_NAME")); - } - - @Test - public void testGenericMustExitBeforeFirstCompleteTuple() throws Exception { - NodeInfo generic = node("one", "", false); - this.registry.register(generic, 3); - Assert.assertThrows(PDException.class, () -> this.registry.register(node("one", "Foo", true), 3)); - Query raw = Query.newBuilder().setAppName(this.app).putLabels("GRAPHSPACE", "team_a") - .putLabels("SERVICE_NAME", "Foo").build(); - Assert.assertEquals(0, this.registry.getNodes(raw).getInfoCount()); - remove(generic); - this.registry.register(node("one", "Foo", true), 3); - Assert.assertEquals(1, this.registry.getNodes(raw).getInfoCount()); - } - - @Test - public void testDirtyPoolRejectsIdQueryAndRenewalButKeepsEnumeration() throws Exception { - // Seed actual legacy writes that predate the authority policy. - DiscoveryMetaStore meta = new DiscoveryMetaStore(this.config); - NodeInfo full = node("one", "Foo", true); - meta.register(full, 3); - meta.register(node("two", "", false).toBuilder().setVersion("v2").build(), 3); - Assert.assertThrows(PDRuntimeException.class, () -> this.registry.getNodes(byId())); - Assert.assertThrows(PDRuntimeException.class, () -> this.registry.getNodes( - byId().toBuilder().setVersion("v1").build())); - Assert.assertThrows(PDException.class, () -> this.registry.register(full, 3)); - Assert.assertEquals(2, this.registry.getNodes(Query.newBuilder().setAppName(this.app).build()).getInfoCount()); - } - - @Test - public void testExpiredGenericDoesNotReserveIdentity() throws Exception { - this.registry.register(node("one", "", false).toBuilder().setInterval(0).build(), 3); - this.registry.register(node("two", "Foo", true), 3); - Assert.assertEquals(1, this.registry.getNodes(byId()).getInfoCount()); - } - - @Test - public void testCompleteEmptyValuesAreComparedWithoutNewInputRules() throws Exception { - NodeInfo empty = node("one", "", true).toBuilder().putLabels("GRAPHSPACE", "").build(); - this.registry.register(empty, 3); - this.registry.register(empty.toBuilder().setAddress("two").build(), 3); - Assert.assertEquals(2, this.registry.getNodes(byId()).getInfoCount()); - } -} diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index b45899964e..800ba6ebed 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -1165,11 +1165,9 @@ private void registerServiceToPd(String graphSpace, Service service) { } else { LOG.warn("RegisterServiceToPd Error. {}", header.getError().getMessage()); - return; } } } - LOG.debug("PD service registration acknowledged"); if (!licenseValid) { LOG.warn("License is valid."); licenseValid = true; @@ -1187,7 +1185,7 @@ private void registerServiceToPd(String graphSpace, Service service) { String pdServiceId = register.registerService(config); service.pdServiceId(pdServiceId); - LOG.info("Scheduled service registration with PD"); + LOG.info("Success to register service to pd"); } catch (Exception e) { LOG.error("Failed to register service to pd", e); From dcf93fd5717f0794b0297674ea11de781a7c2eb8 Mon Sep 17 00:00:00 2001 From: imbajin Date: Thu, 15 Oct 2026 07:50:53 +0800 Subject: [PATCH 11/11] docs(server): clarify normalization owner - attribute SERVICE_ID normalization to GraphManager - align Docker and Helm service identity guidance - preserve existing discovery and collision contracts --- docker/README.md | 8 ++++---- helm/hugegraph/README.md | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docker/README.md b/docker/README.md index 2ba2bac15a..233cd4ef71 100644 --- a/docker/README.md +++ b/docker/README.md @@ -301,10 +301,10 @@ local Compose override. The Helm chart exposes the same settings as Use a chart and Server image from the same source version containing these mappings. Older release images without them do not apply these environment values; select the matching image build explicitly. -PD's existing `SERVICE_ID` normalization is unchanged and can map different raw -names to the same identifier. Select `GRAPHSPACE` and the complete raw -`SERVICE_NAME` in discovery queries; identifier-only lookup does not provide a -uniqueness or isolation guarantee. +The Server's existing `SERVICE_ID` normalization (`GraphManager.serviceId()`) is +unchanged and can map different raw names to the same identifier. Select +`GRAPHSPACE` and the complete raw `SERVICE_NAME` in discovery queries; +identifier-only lookup does not provide a uniqueness or isolation guarantee. This selects the Server's PD service metadata. It does not change `server.path_graphspace` for REST paths that omit `/graphspaces/`, or diff --git a/helm/hugegraph/README.md b/helm/hugegraph/README.md index c6c62c1ed9..a65039558f 100644 --- a/helm/hugegraph/README.md +++ b/helm/hugegraph/README.md @@ -117,9 +117,9 @@ helm upgrade --install hugegraph ./helm/hugegraph -n hugegraph \ Use a chart and Server image build from the same source version containing these mappings. Older release images without them ignore these environment settings; select a matching image build explicitly. The chart does not detect old images. -PD's existing `SERVICE_ID` normalization is unchanged: select `GRAPHSPACE` and the -complete raw `SERVICE_NAME` in discovery queries. Identifier-only lookup has no -uniqueness or isolation guarantee. +The Server's existing `SERVICE_ID` normalization (`GraphManager.serviceId()`) is +unchanged: select `GRAPHSPACE` and the complete raw `SERVICE_NAME` in discovery +queries. Identifier-only lookup has no uniqueness or isolation guarantee. `server.testGraphAlias` selects an existing Gremlin binding for every Server Pod, defaulting to `DEFAULT-hugegraph`. It does not create a graph or infer its binding