snleee commented on code in PR #9295: URL: https://github.com/apache/pinot/pull/9295#discussion_r964338431
########## pinot-plugins/pinot-batch-ingestion/pinot-batch-ingestion-standalone/src/main/java/org/apache/pinot/plugin/ingestion/batch/standalone/SegmentTarPushJobRunner.java: ########## @@ -46,48 +35,8 @@ public SegmentTarPushJobRunner(SegmentGenerationJobSpec spec) { init(spec); } - @Override - public void init(SegmentGenerationJobSpec spec) { - _spec = spec; - } - - @Override - public void run() { - //init all file systems - List<PinotFSSpec> pinotFSSpecs = _spec.getPinotFSSpecs(); - for (PinotFSSpec pinotFSSpec : pinotFSSpecs) { - PinotFSFactory.register(pinotFSSpec.getScheme(), pinotFSSpec.getClassName(), new PinotConfiguration(pinotFSSpec)); - } - - //Get outputFS for writing output pinot segments - URI outputDirURI; - try { - outputDirURI = new URI(_spec.getOutputDirURI()); - if (outputDirURI.getScheme() == null) { - outputDirURI = new File(_spec.getOutputDirURI()).toURI(); - } - } catch (URISyntaxException e) { - throw new RuntimeException("outputDirURI is not valid - '" + _spec.getOutputDirURI() + "'"); - } - PinotFS outputDirFS = PinotFSFactory.create(outputDirURI.getScheme()); - //Get list of files to process - String[] files; - try { - files = outputDirFS.listFiles(outputDirURI, true); - } catch (IOException e) { - throw new RuntimeException("Unable to list all files under outputDirURI - '" + outputDirURI + "'"); - } - - List<String> segmentsToPush = new ArrayList<>(); - for (String file : files) { - if (file.endsWith(Constants.TAR_GZ_FILE_EXT)) { - segmentsToPush.add(file); - } - } - try { - SegmentPushUtils.pushSegments(_spec, outputDirFS, segmentsToPush); - } catch (RetriableOperationException | AttemptsExceededException e) { - throw new RuntimeException(e); - } + public void uploadSegments(Map<String, String> segmentsUriToTarPathMap) + throws AttemptsExceededException, RetriableOperationException { + SegmentPushUtils.pushSegments(_spec, _outputDirFS, new ArrayList<>(segmentsUriToTarPathMap.values())); Review Comment: For copying the elements from `map.values()` to `List`? In our case, the worst-case scenario should be 10-100 thousand and I think it's OK to keep it this way. I don't think that it's a performance-critical path. -- 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