jugomezv commented on code in PR #9994:
URL: https://github.com/apache/pinot/pull/9994#discussion_r1055627527


##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/SegmentOnlineOfflineStateModelFactory.java:
##########
@@ -91,6 +92,16 @@ public void onBecomeOnlineFromConsuming(Message message, 
NotificationContext con
 
       TableDataManager tableDataManager = 
_instanceDataManager.getTableDataManager(realtimeTableName);
       Preconditions.checkNotNull(tableDataManager);
+      if (tableDataManager instanceof RealtimeTableDataManager) {

Review Comment:
   I can do this but I don't like the way the code ends up looking like: since 
we are talking about a TableDataManager and not a BaseTableDataManager, I will 
still have to cast TableDataManager to BaseTableDataManager before I invoke the 
method. (Note the hierarchy is as follows 
TableDataManager->BaseTableDataManager->RealTimeTableDataManager)  As an 
alternative I can bring these interfaces up to TableDataManager and override in 
BaseDataManager as nop function and then with the implementation I need in 
RealTimeTableDataManager. This has the problem that we start exposing low level 
interfaces in what should be the high level TableDataManager. So I think it is 
best to leave the code as is.



-- 
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.

To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org

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