dongxiaoman commented on a change in pull request #7064:
URL: https://github.com/apache/incubator-pinot/pull/7064#discussion_r654065894
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
##########
@@ -330,6 +334,27 @@ public void start()
LOGGER.info("Finish starting Pinot broker");
}
+ @VisibleForTesting
+ static void updateHelixHost(PinotConfiguration brokerConf, HelixManager
helixManager, String clusterName, String brokerInstanceId) {
+ boolean allowUpdateHelixHost =
brokerConf.getProperty(Broker.BROKER_DYNAMIC_HELIX_HOST, false);
Review comment:
Removed.
##########
File path:
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
##########
@@ -199,6 +199,9 @@
public static final String BROKER_TLS_PREFIX = "pinot.broker.tls";
public static final String BROKER_NETTYTLS_ENABLED =
"pinot.broker.nettytls.enabled";
+ public static final String BROKER_DYNAMIC_HELIX_HOST =
"pinot.broker.dynamic.helix.host";
+ public static final String BROKER_NETTY_PORT = "pinot.broker.netty.port";
+ public static final String BROKER_NETTY_HOST = "pinot.broker.netty.host";
Review comment:
let me double check that; forgot this comment
##########
File path:
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
##########
@@ -199,6 +199,9 @@
public static final String BROKER_TLS_PREFIX = "pinot.broker.tls";
public static final String BROKER_NETTYTLS_ENABLED =
"pinot.broker.nettytls.enabled";
+ public static final String BROKER_DYNAMIC_HELIX_HOST =
"pinot.broker.dynamic.helix.host";
+ public static final String BROKER_NETTY_PORT = "pinot.broker.netty.port";
+ public static final String BROKER_NETTY_HOST = "pinot.broker.netty.host";
Review comment:
Removed now
##########
File path:
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
##########
@@ -396,6 +399,8 @@
// but we keep this default for backward compatibility in case someone
relies on this format
// see Server or Broker class for correct prefix format you should use
public static final String DEFAULT_METRICS_PREFIX = "pinot.controller.";
+ public static final String CONTROLLER_DYNAMIC_HELIX_HOST =
"pinot.controller.dynamic.helix.host";
Review comment:
Removed
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
##########
@@ -319,6 +322,8 @@ public void start()
new BrokerUserDefinedMessageHandlerFactory(_routingManager,
queryQuotaManager));
_participantHelixManager.connect();
addInstanceTagIfNeeded();
+ updateHelixHost(_brokerConf, _participantHelixManager, _clusterName,
_brokerId);
Review comment:
Yes, a good point; I refactored the code so now the 3 host types are
using the same function now.
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/MinionStarter.java
##########
@@ -225,7 +226,13 @@ public void start()
_helixManager.getStateMachineEngine().registerStateModelFactory("Task",
new TaskStateModelFactory(_helixManager,
new TaskFactoryRegistry(_taskExecutorFactoryRegistry,
_eventObserverFactoryRegistry).getTaskFactoryRegistry()));
_helixManager.connect();
- addInstanceTagIfNeeded();
+ HelixHelper.updateInstanceConfigIfNeeded(
Review comment:
please double check; I don't have minions setup so cannot verify it.
--
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
