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


##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompactmerge/UpsertCompactMergeTaskGenerator.java:
##########
@@ -462,4 +472,54 @@ protected String 
getSegmentCrcList(List<SegmentMergerMetadata> segmentMergerMeta
         segmentMergerMetadataList.stream().map(x -> 
String.valueOf(x.getSegmentZKMetadata().getCrc()))
             .collect(Collectors.toList()), ",");
   }
+
+  /**
+   * Retrieves the maximum creation time (in milliseconds) among the specified 
segments across all servers.
+   *
+   * <p>This method filters the provided server-to-segments map to only 
include the segments specified in
+   * {@code segmentNames}, then queries the servers for the creation time 
metadata of those segments using
+   * {@link 
ServerSegmentMetadataReader#getSegmentCreationMetadataFromServers}. It returns 
the maximum creation
+   * time found. If no creation time is found for the given segments, a {@link 
RuntimeException} is thrown.
+   *
+   * @param tableNameWithType The name of the table with type (e.g., 
myTable_OFFLINE)
+   * @param segmentNames The list of segment names to query
+   * @param serverToSegments A map from server instance to the list of 
segments it hosts
+   * @param serverToEndpoints A BiMap from server instance to its admin 
endpoint
+   * @param serverSegmentMetadataReader The reader to fetch segment metadata 
from servers
+   * @return The maximum creation time in milliseconds among the specified 
segments
+   * @throws RuntimeException If no creation time is found for the given 
segments or if an I/O error occurs
+   */
+  @VisibleForTesting
+  protected Long getMaxCreationTimeMillis(String tableNameWithType, 
List<String> segmentNames,
+      Map<String, List<String>> serverToSegments, BiMap<String, String> 
serverToEndpoints,
+      ServerSegmentMetadataReader serverSegmentMetadataReader) {
+
+    // Filter serverToSegments to only include segments present in segmentNames
+    Set<String> segmentNameSet = new HashSet<>(segmentNames);
+    Map<String, List<String>> filteredServerToSegments = new HashMap<>();
+    for (Map.Entry<String, List<String>> entry : serverToSegments.entrySet()) {
+      List<String> filteredSegments =
+          
entry.getValue().stream().filter(segmentNameSet::contains).collect(Collectors.toList());
+      if (!filteredSegments.isEmpty()) {
+        filteredServerToSegments.put(entry.getKey(), filteredSegments);
+      }
+    }
+    Map<String, List<Long>> creationTimeMap;
+    try {
+      creationTimeMap =
+          
serverSegmentMetadataReader.getSegmentCreationMetadataFromServers(tableNameWithType,
 filteredServerToSegments,

Review Comment:
   IIUC in this method `getSegmentToValidDocIdsMetadataFromServer` we are 
making a call to all the servers to fetch metadata for all the segments on that 
server. And if you check the method we are already parsing `creation.meta` 
there once to get the `crc` value of that segment. We had added crc to the 
`ValidDocIdsMetadataInfo` while working on `crc` mismatch inconsistency bug. We 
can just extend that itself to add creationTime itself as we are already 
parsing creation.meta there and save more calls to servers (even if they are 
just few. We can make it zero so why not!).



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