[GitHub] [pinot] walterddr commented on a diff in pull request #10279: move segment existency check to preprocess

["walterddr (via GitHub)" <gi...@apache.org>]

walterddr commented on code in PR #10279:
URL: https://github.com/apache/pinot/pull/10279#discussion_r1106160968


##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/BaseMultipleSegmentsConversionExecutor.java:
##########
@@ -107,14 +107,31 @@ protected abstract List<SegmentConversionResult> convert(PinotTaskConfig pinotTa
 
   /**
    * Pre processing operations to be done at the beginning of task execution
+   *
+   * The default implementation checks whether all segments to process exist in the table, if not, terminate early to
+   * avoid wasting computing resources.
    */
-  protected void preProcess(PinotTaskConfig pinotTaskConfig) {
+  protected void preProcess(PinotTaskConfig pinotTaskConfig) throws Exception {
+    Map<String, String> configs = pinotTaskConfig.getConfigs();
+    String tableNameWithType = configs.get(MinionConstants.TABLE_NAME_KEY);
+    String inputSegmentNames = configs.get(MinionConstants.SEGMENT_NAME_KEY);
+    String uploadURL = configs.get(MinionConstants.UPLOAD_URL_KEY);
+    AuthProvider authProvider = AuthProviderUtils.makeAuthProvider(configs.get(MinionConstants.AUTH_TOKEN));
+    Set<String> segmentNamesForTable = SegmentConversionUtils.getSegmentNamesForTable(tableNameWithType,
+        FileUploadDownloadClient.extractBaseURI(new URI(uploadURL)), authProvider);
+    Set<String> nonExistingSegmentNames =
+        new HashSet<>(Arrays.asList(inputSegmentNames.split(MinionConstants.SEGMENT_NAME_SEPARATOR)));
+    nonExistingSegmentNames.removeAll(segmentNamesForTable);
+    if (!CollectionUtils.isEmpty(nonExistingSegmentNames)) {
+      throw new RuntimeException(String.format("table: %s does have the following segments to process: %s",
+          tableNameWithType, nonExistingSegmentNames));
+    }
   }
 
   /**
    * Post processing operations to be done before exiting a successful task execution
    */
-  protected void postProcess(PinotTaskConfig pinotTaskConfig) {
+  protected void postProcess(PinotTaskConfig pinotTaskConfig) throws Exception {
   }

Review Comment:
   is this change needed?



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