FrankChen021 commented on PR #19598: URL: https://github.com/apache/druid/pull/19598#issuecomment-5390045745
Thanks for adding this. After tracing the usage, I think the scope needs clarification and possibly additional changes. The new property is applied indirectly through `DruidNode.getHostAndPort()` (and therefore `getHostAndPortToUse()` when Druid TLS is disabled), and directly through `ServiceLocation.fromDruidNode()`. However, `DruidNode.getPortToUse()` and `getUriToUse()` still return/use `plaintextPort`, and there are peer-facing paths that use those methods, including basic-security cache notifications, catalog synchronization, the system server-properties fetch, and Consul registration. If the goal is for peer traffic to go through the sidecar, could you please review these call sites and either: 1. use the advertised port where appropriate, or 2. document the narrower set of discovery/RPC paths this property is intended to affect and explain why the remaining paths should continue using the bind port? Could you also update the configuration documentation to include `druid.advertisedPlaintextPort` alongside `druid.plaintextPort`? The documentation should ideally state: - `plaintextPort` remains the Jetty bind/listen port; - `advertisedPlaintextPort` is the port published/used by the supported discovery and RPC paths; - when unset or non-positive, it falls back to `plaintextPort`; - it applies only when plaintext is enabled, and Druid's configured TLS port remains preferred when TLS is enabled. Without this documentation, it is difficult for operators to discover the property or understand its interaction with the existing port settings. -- 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]
