bitflicker64 commented on code in PR #3187:
URL: https://github.com/apache/hugegraph/pull/3187#discussion_r3939609317


##########
hugegraph-server/hugegraph-dist/docker/README.md:
##########
@@ -144,3 +144,30 @@ native `HEALTHCHECK` instructions. `docker ps` shows real 
health status:
 | `hugegraph/hugegraph-store` | `GET /v1/health` on port 8520 |
 
 The entrypoints supervise the Java process directly — when Java exits, the 
container exits. If started with a restart policy (the provided compose files 
use `restart: unless-stopped`), Docker will bring it back automatically. The 
old cron-based monitor (`-m true`) is for VM/bare-metal deployments only and is 
not used in Docker images.
+
+## 7. Server Startup Timeout
+
+The entrypoint gives the Server a fixed budget to answer on its REST port and
+ends the container when the budget runs out. It is 120 seconds by default. Set
+`HG_SERVER_STARTUP_TIMEOUT_S` to a whole number of seconds between 1 and 86400

Review Comment:
   Fixed in 0ba30a1e7. Confirmed the reproduction first: `docker compose -f 
docker/docker-compose.yml config` emitted no such key.
   
   All three topologies now carry `HG_SERVER_STARTUP_TIMEOUT_S: 
${HG_SERVER_STARTUP_TIMEOUT_S-120}`, unset-only as you asked, so an absent host 
value renders 120 while an explicitly empty one reaches the entrypoint and is 
rejected there instead of silently becoming the default. The HA file gets it 
through the `x-server-environment` anchor, so all three Servers share it.
   
   `test-compose.sh render` now asserts the default and a 450 override on every 
Server service, and `compose_auth` strips the variable so the baseline render 
cannot be coloured by the developer's environment. Verified the assertion fails 
when the key is removed from a Compose file.



##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh:
##########
@@ -232,4 +233,60 @@ rm -f "${TEST_HOME}/docker/init_complete"
 )
 grep -Fqx -- '-n' "${TEST_HOME}/docker/init-store-password"
 
+last_start_args() { tail -n 1 "${TEST_HOME}/docker/start-hugegraph-args"; }
+
+[[ "$(last_start_args)" == *"-t 120"* ]]
+
+(
+    cd "${TEST_HOME}"
+    HG_SERVER_STARTUP_TIMEOUT_S=450 bash ./docker-entrypoint.sh
+)
+[[ "$(last_start_args)" == *"-t 450"* ]]

Review Comment:
   Fixed in 0ba30a1e7. The positive cases now use an `assert_start_timeout` 
helper that exits explicitly and prints the value it saw, so bash 3.2 no longer 
reaches PASS through a failing `[[ ]]`.
   
   Verified on 3.2.57: changing the entrypoint default to 999 now reports 
`expected start-hugegraph.sh -t 120, got -t 999` instead of passing. The suite 
passes on both 3.2.57 and 5.3.15.



##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh:
##########
@@ -232,4 +233,60 @@ rm -f "${TEST_HOME}/docker/init_complete"
 )
 grep -Fqx -- '-n' "${TEST_HOME}/docker/init-store-password"
 
+last_start_args() { tail -n 1 "${TEST_HOME}/docker/start-hugegraph-args"; }
+
+[[ "$(last_start_args)" == *"-t 120"* ]]

Review Comment:
   Fixed in 0ba30a1e7. The stub now writes `printf '%s\n' "$@" > 
./docker/start-hugegraph-argv`, one argument per line, and `last_start_timeout` 
walks that vector to return the exact argument following `-t`. Assertions 
compare it with `!=` rather than a substring.
   
   Verified with your case: making the entrypoint pass `-t 1200` now reports 
`expected start-hugegraph.sh -t 120, got -t 1200`, where the old substring 
check passed. Call counting moved to a separate `start-hugegraph-calls` log so 
the argv file can be overwritten per call.



##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint-test.sh:
##########
@@ -232,4 +233,60 @@ rm -f "${TEST_HOME}/docker/init_complete"
 )
 grep -Fqx -- '-n' "${TEST_HOME}/docker/init-store-password"
 
+last_start_args() { tail -n 1 "${TEST_HOME}/docker/start-hugegraph-args"; }
+
+[[ "$(last_start_args)" == *"-t 120"* ]]
+
+(
+    cd "${TEST_HOME}"
+    HG_SERVER_STARTUP_TIMEOUT_S=450 bash ./docker-entrypoint.sh
+)
+[[ "$(last_start_args)" == *"-t 450"* ]]
+
+(
+    cd "${TEST_HOME}"
+    HG_SERVER_STARTUP_TIMEOUT_S=86400 bash ./docker-entrypoint.sh
+)
+[[ "$(last_start_args)" == *"-t 86400"* ]]
+
+# An empty value is a set value, not an absent one: Compose writes it whenever
+# an interpolated host variable is missing. 2m is the shape of a typo, and the
+# two large values bracket the point where the deadline arithmetic in
+# wait_for_startup would wrap negative and end the wait before its first probe.
+for invalid_timeout in "" " " 0 +5 2m 86401 9223372036854775807; do
+    start_calls_before_invalid=$(wc -l < 
"${TEST_HOME}/docker/start-hugegraph-args")
+    init_calls_before_invalid=$(wc -l < "${TEST_HOME}/docker/init-store-calls")
+    if (
+        cd "${TEST_HOME}"
+        HG_SERVER_STARTUP_TIMEOUT_S="${invalid_timeout}" \
+            bash ./docker-entrypoint.sh
+    ); then
+        echo "startup timeout '${invalid_timeout}' unexpectedly succeeded" >&2
+        exit 1
+    fi
+    # The server must not have started, and the guard must have run ahead of
+    # init-store, as the comment above it in the entrypoint claims. Spelled
+    # with an explicit exit rather than a bare [[ ]]: bash 3.2, still the
+    # /bin/bash of macOS, does not apply set -e to a failing [[ ]], so a bare
+    # assertion passes silently there while CI catches the regression.
+    [[ "$(wc -l < "${TEST_HOME}/docker/start-hugegraph-args")" -eq \
+       "${start_calls_before_invalid}" ]] || {
+        echo "startup timeout '${invalid_timeout}' started the server" >&2
+        exit 1
+    }
+    [[ "$(wc -l < "${TEST_HOME}/docker/init-store-calls")" -eq \
+       "${init_calls_before_invalid}" ]] || {
+        echo "startup timeout '${invalid_timeout}' was rejected only after" \
+             "init-store ran" >&2
+        exit 1
+    }
+done
+
+# An unset variable still keeps the historical default.
+(
+    cd "${TEST_HOME}"
+    bash ./docker-entrypoint.sh

Review Comment:
   Fixed in 0ba30a1e7. Both unset cases now run under `env -u 
HG_SERVER_STARTUP_TIMEOUT_S`, and the assertion is the explicit helper 
described above rather than a bare `[[ ]]`.
   
   Verified: with `HG_SERVER_STARTUP_TIMEOUT_S=450` exported, the suite still 
passes, and reverting the `env -u` reports `expected start-hugegraph.sh -t 120, 
got -t 450` on bash 3.2.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to