Jackie-Jiang commented on a change in pull request #4089: In
HelixBrokerStarter, allow custom cluster change handlers
URL: https://github.com/apache/incubator-pinot/pull/4089#discussion_r273319943
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
##########
@@ -226,6 +232,45 @@ public void run() {
return brokerServerBuilder;
}
+ /**
+ * To be overridden to plug in custom external view change handlers.
+ * <p>NOTE: all change handlers will be run in a single thread, so any slow
change handler can block other change
+ * handlers from running. For slow change handler, make it asynchronous.
+ *
+ * @param spectatorHelixManager Spectator Helix manager
+ * @return List of custom external view change handlers to plug in
+ */
+ @SuppressWarnings("unused")
+ protected List<ClusterChangeHandler>
getCustomExternalViewChangeHandlers(HelixManager spectatorHelixManager) {
Review comment:
I'm not a big fan of using Class.newInstance() because of the reasons here:
https://stackoverflow.com/questions/4612386/what-is-the-difference-between-the-new-operator-and-class-newinstance
Also, it might be hard to setup handlers with zero-argument constructor if
user want to pass in some properties not in open source code.
Currently we have both flavors of code in the code base. We can discuss this
offline.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
