apucher commented on a change in pull request #6418:
URL: https://github.com/apache/incubator-pinot/pull/6418#discussion_r557833005
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
##########
@@ -117,13 +124,18 @@ public HelixBrokerStarter(PinotConfiguration brokerConf,
String clusterName, Str
brokerHost =
_brokerConf.getProperty(CommonConstants.Helix.SET_INSTANCE_ID_TO_HOSTNAME_KEY,
false) ? NetUtil
.getHostnameOrAddress() : NetUtil.getHostAddress();
}
+
_brokerId = _brokerConf.getProperty(Helix.Instance.INSTANCE_ID_KEY,
- Helix.PREFIX_OF_BROKER_INSTANCE + brokerHost + "_" + _brokerConf
- .getProperty(Helix.KEY_OF_BROKER_QUERY_PORT,
Helix.DEFAULT_BROKER_QUERY_PORT));
+ Helix.PREFIX_OF_BROKER_INSTANCE + brokerHost + "_" + inferPort());
Review comment:
We'll pick whichever port is configured first - which is usually the
HTTP port, even in a multi-ingress scenario. If only HTTPS is enabled then that
explicitly-configured port will be used for the instance id.
Is this what you suggest I note in the release-notes?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
