imbajin commented on code in PR #3052:
URL: https://github.com/apache/hugegraph/pull/3052#discussion_r3368213456


##########
hugegraph-server/Dockerfile:
##########
@@ -67,5 +66,9 @@ RUN chmod 755 ./docker-entrypoint.sh
 EXPOSE 8080
 VOLUME /hugegraph-server
 
+HEALTHCHECK --interval=15s --timeout=10s --start-period=90s --retries=3 \
+    CMD curl -fsS http://localhost:8080/versions >/dev/null \
+        || kill -0 "$(cat ./bin/pid 2>/dev/null)" 2>/dev/null

Review Comment:
   ⚠️ **Keep the healthcheck tied to the HTTP endpoint**
   
   Evidence: the existing compose healthchecks use `curl ... || exit 1` for 
`/versions` and `/v1/health`, but the new Dockerfile healthchecks return 
success whenever `./bin/pid` still points to a live JVM. This same fallback is 
added to the server, hstore, PD, and store images.
   
   Impact: after the start period, a container can remain `healthy` even when 
the REST endpoint is down or wedged, which weakens the health signal this PR is 
adding. Please make the steady-state Dockerfile healthcheck fail on HTTP 
failure, or keep any process-only fallback outside the steady-state HEALTHCHECK 
path.



-- 
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