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

Reply via email to