zhtaoxiang commented on code in PR #10034:
URL: https://github.com/apache/pinot/pull/10034#discussion_r1059725960
##########
pinot-plugins/pinot-batch-ingestion/pinot-batch-ingestion-hadoop/src/main/java/org/apache/pinot/plugin/ingestion/batch/hadoop/HadoopSegmentCreationMapper.java:
##########
@@ -183,13 +184,26 @@ protected void map(LongWritable key, Text value, Context
context) {
LOGGER.info("Size for segment: {}, uncompressed: {}, compressed: {}",
segmentName,
DataSizeUtils.fromBytes(uncompressedSegmentSize),
DataSizeUtils.fromBytes(compressedSegmentSize));
//move segment to output PinotFS
- URI outputSegmentTarURI =
SegmentGenerationUtils.getRelativeOutputPath(inputDirURI, inputFileURI,
outputDirURI)
- .resolve(segmentTarFileName);
- LOGGER.info("Copying segment tar file from [{}] to [{}]",
localSegmentTarFile, outputSegmentTarURI);
- outputDirFS.copyFromLocalFile(localSegmentTarFile, outputSegmentTarURI);
+ URI relativeOutputPath =
SegmentGenerationUtils.getRelativeOutputPath(inputDirURI, inputFileURI,
outputDirURI);
+ URI outputSegmentTarURI = relativeOutputPath.resolve(segmentTarFileName);
+ SegmentGenerationJobUtils.moveLocalTarFileToRemote(localSegmentTarFile,
outputSegmentTarURI,
+ _spec.isOverwriteOutput());
+
+ // Create and upload segment metadata tar file
+ String metadataTarFileName = URLEncoder.encode(segmentName +
Constants.METADATA_TAR_GZ_FILE_EXT, "UTF-8");
+ URI outputMetadataTarURI =
relativeOutputPath.resolve(metadataTarFileName);
+ if (_spec.isOverwriteOutput() &&
outputDirFS.exists(outputMetadataTarURI)) {
Review Comment:
Thanks for the udpate!
I feel that there are 3 cases
1. `_spec.isOverwriteOutput()` is true -> we need to delete metadata from
output dir when it exists
2. `_spec.isOverwriteOutput()` is false and `taskSpec.isCreateMetadataTarGz`
is true, we should try to create metadata targz locally and try to copy it to
output dir in case the metadata targz does not exist (due to user switches off
then on the `isCreateMetadataTarGz` flag)
3. `_spec.isOverwriteOutput()` is false and `taskSpec.isCreateMetadataTarGz`
is false, we should try to delete metadata targz from output dir in case the
metadata targz exists (due to user switches on then off the
`isCreateMetadataTarGz` flag)
I think we missed case 3, what do you think?
--
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
