[GitHub] [pinot] mcvsubbu commented on a change in pull request #7743: Always use SMALLEST offset criteria for new detected partitions

[GitBox <gi...@apache.org>]

mcvsubbu commented on a change in pull request #7743:
URL: https://github.com/apache/pinot/pull/7743#discussion_r747175821



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
##########
@@ -872,8 +872,14 @@ public void ensureAllPartitionsConsuming(TableConfig tableConfig, PartitionLevel
       if (idealState.isEnabled()) {
         List<PartitionGroupConsumptionStatus> currentPartitionGroupConsumptionStatusList =
             getPartitionGroupConsumptionStatusList(idealState, streamConfig);
+
+        // Read the smallest offset when a new partition is detected
+        OffsetCriteria originalOffsetCriteria = streamConfig.getOffsetCriteria();
+        streamConfig.setOffsetCriteria(OffsetCriteria.SMALLEST_OFFSET_CRITERIA);

Review comment:
       Is the table config being pulled from cache? If so, will we be modifying the streamconfig of that tableconfig? Or, if someone re-uses it down the road and gets the wrong value in different part of the code.
   
   I think a better way may be to update the setupNewPartitionGroup() method to take an additional boolean arg isLiveTable. If it is a live table, then pick smallest offset criteria




-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org