tarun11Mavani commented on code in PR #15863:
URL: https://github.com/apache/pinot/pull/15863#discussion_r2105785850


##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompactmerge/UpsertCompactMergeTaskGenerator.java:
##########
@@ -214,15 +216,24 @@ public List<PinotTaskConfig> 
generateTasks(List<TableConfig> tableConfigs) {
           continue;
         }
         // TODO see if multiple groups of same partition can be added
-        Map<String, String> configs = new 
HashMap<>(getBaseTaskConfigs(tableConfig,
-            groups.get(0).stream().map(x -> 
x.getSegmentZKMetadata().getSegmentName()).collect(Collectors.toList())));
+
+        List<String> segmentNames =
+            groups.get(0).stream().map(x -> 
x.getSegmentZKMetadata().getSegmentName()).collect(Collectors.toList());
+        //get max creation time for the segments across all servers. This will 
be used as the creation time of the
+        // merge segment
+        Long maxCreationTimeMillis =
+            getMaxCreationTimeMillis(tableNameWithType, segmentNames, 
serverToSegments, serverToEndpoints,

Review Comment:
   > Let's emit a metric when newest merging segment and compacted segment are 
of UploadedRealtimeSegment.
   
   I am concerned on emitting too many metrics in this case. Say we are 
replacing a UploadedRealtimeSegment with another UploadedRealtimeSegment with 
same creation time. We will end up emitting metric for each doc, is it okay to 
emit so many metric? 
   
   > Should we also consider skipping a task where a set of segments is 
selected where the newest segment is already a compacted segment so this 
scenario does not happen.
   
   Yeah. That's a good suggestion. Will try to add this.



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