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


##########
hugegraph-store/docs/deployment-guide.md:
##########
@@ -719,7 +719,7 @@ environment:
 ```
 
 **Startup ordering** is enforced via `depends_on` with `condition: 
service_healthy`:
-1. PD nodes start first and must pass healthchecks (`/v1/health`)
+1. PD nodes start first and must pass healthchecks (`/v1/ready`, answered 
`200` only once the PD sees a raft leader)

Review Comment:
   Good catch, fixed in ffa13f9. This line was left behind when c8adc85 
reverted both compose healthchecks to `/v1/health`, so the guide described a 
gate the repository does not have.
   
   Took your first option. Step 1 now reads `/v1/health`, liveness only, 
followed by a sentence saying it answers `200` before a quorum forms and 
pointing at `docker/README.md` for what switching the healthchecks to 
`/v1/ready` requires: a body match on `"ready":true`, because PD answers `200` 
on any non-excluded path, plus an image carrying the endpoint.
   
   Swept the rest of the diff for the same mistake. The only other mentions are 
the endpoint reference itself and operator guidance, not claims about these 
compose files.



##########
hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java:
##########
@@ -32,6 +32,7 @@ public class AuthenticationConfigurer implements 
WebMvcConfigurer {
     public void addInterceptors(InterceptorRegistry registry) {
         registry.addInterceptor(restAuthentication)
                 .addPathPatterns("/**")
-                .excludePathPatterns("/actuator/*", "/v1/health", 
"/v1/prom/targets/*");
+                .excludePathPatterns("/actuator/*", "/v1/health", "/v1/ready",
+                                     "/v1/prom/targets/*");

Review Comment:
   Settling on the shape in c8adc85, so resolving this. The address, the one 
thing anonymous callers could not learn before, is gone; `ready`, `state` and 
`isLeader` stay.
   
   To recap the reasoning: `ready` has to be in the body because PD answers 
`200` on rejection, so a status-only probe cannot gate on it. `isLeader` 
duplicates `hg_raft_leader`, which is anonymous on `/actuator/prometheus` by 
design. `state` is what makes a `503` diagnosable without shell access to the 
pod.
   
   Reopen if you would rather it were `ready` alone; that would still leave the 
compose gate workable.



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