[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10874: Making segmentMapper do the init and cleanup of RecordReader

["Jackie-Jiang (via GitHub)" <gi...@apache.org>]

Jackie-Jiang commented on code in PR #10874:
URL: https://github.com/apache/pinot/pull/10874#discussion_r1228656776


##########
pinot-core/src/main/java/org/apache/pinot/core/segment/processing/framework/SegmentProcessorFramework.java:
##########
@@ -72,11 +73,34 @@ public SegmentProcessorFramework(List<RecordReader> recordReaders, SegmentProces
       File workingDir)
       throws IOException {
     Preconditions.checkState(!recordReaders.isEmpty(), "No record reader is provided");
-
     LOGGER.info("Initializing SegmentProcessorFramework with {} record readers, config: {}, working dir: {}",
         recordReaders.size(), segmentProcessorConfig, workingDir.getAbsolutePath());
 
-    _recordReaders = recordReaders;
+    // Populate the RecordReaderFileConfig (container for input), so that we deal with just one data structure.
+    _recordReaderFileConfigs = new ArrayList<>();
+    for (RecordReader recordReader : recordReaders) {
+      _recordReaderFileConfigs.add(new RecordReaderFileConfig(recordReader));
+    }
+
+    _segmentProcessorConfig = segmentProcessorConfig;
+
+    _mapperOutputDir = new File(workingDir, "mapper_output");
+    FileUtils.forceMkdir(_mapperOutputDir);
+    _reducerOutputDir = new File(workingDir, "reducer_output");
+    FileUtils.forceMkdir(_reducerOutputDir);
+    _segmentsOutputDir = new File(workingDir, "segments_output");
+    FileUtils.forceMkdir(_segmentsOutputDir);
+  }
+
+  public SegmentProcessorFramework(SegmentProcessorConfig segmentProcessorConfig, File workingDir,
+      List<RecordReaderFileConfig> recordReaderFileConfigs)

Review Comment:
   (nit) Make `recordReaderFileConfigs` the first argument



##########
pinot-core/src/main/java/org/apache/pinot/core/segment/processing/framework/SegmentProcessorFramework.java:
##########
@@ -72,11 +73,34 @@ public SegmentProcessorFramework(List<RecordReader> recordReaders, SegmentProces
       File workingDir)
       throws IOException {

Review Comment:
   We may introduce a helper method to convert `List<RecordReader>` to `List<RecordReaderFileConfig> recordReaderFileConfigs` and directly call the other constructor.



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