mcvsubbu commented on a change in pull request #5459: URL: https://github.com/apache/incubator-pinot/pull/5459#discussion_r432213242
########## File path: pinot-broker/src/main/java/org/apache/pinot/broker/routing/RoutingManager.java ########## @@ -51,7 +51,7 @@ import org.apache.pinot.common.metrics.BrokerMetrics; import org.apache.pinot.common.request.BrokerRequest; import org.apache.pinot.common.utils.CommonConstants; -import org.apache.pinot.common.utils.CommonConstants.Helix.StateModel.RealtimeSegmentOnlineOfflineStateModel; +import org.apache.pinot.common.utils.CommonConstants.Helix.StateModel.SegmentOnlineOfflineStateModel; Review comment: I dont think merging BrokerStateModel and SegmentStateModel is a good idea. Broker does not have CONSUMING state. I think the reason these are coded separately (for realtime and offline) is because we did not have CONSUMING state before. By now, the state model in helix should have consuming for all pinot use cases. So, merging these is fine. ---------------------------------------------------------------- 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