FrankChen021 commented on code in PR #20806:
URL: https://github.com/apache/druid/pull/20806#discussion_r4450395764
##########
extensions-core/hdfs-storage/src/main/java/org/apache/druid/storage/hdfs/HdfsDataSegmentPusher.java:
##########
@@ -227,6 +257,82 @@ private DataSegment pushToFilePath(File inDir, DataSegment
segment, String outIn
return dataSegment;
}
+ private DataSegment pushToFilePathNone(File inDir, DataSegment segment,
String outIndexDirPath) throws IOException
Review Comment:
this method duplicates the code of `pushToFilePath`.
HDFS FileSystem class provides `copyFromLocalFile` to do copy. we can do it
inside `pushToFilePath` as:
```java
if (compressionFormat == CompressionUtils.Format.NONE) {
fs.copyFromLocalFile(false, true, new Path(inDir.toURI()), tmpPath);
size = FileUtils.getFileSize(inDir);
} else {
try (FSDataOutputStream out = fs.create(tmpPath)) {
size = compressionFormat.compressDirectory(inDir, out);
}
}
```
##########
extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentPusher.java:
##########
@@ -62,7 +62,14 @@ public S3DataSegmentPusher(
{
this.s3Client = s3Client;
this.config = config;
- this.zip = deepStorageConfig.isZip(DEFAULT_ZIP);
+ final CompressionUtils.Format format =
deepStorageConfig.getEffectiveCompressionFormat(DEFAULT_FORMAT);
+ if (format != CompressionUtils.Format.ZIP && format !=
CompressionUtils.Format.NONE) {
+ throw DruidException.defensive(
Review Comment:
other classes throw IAE. please align the exception.
SInce the `getEffectiveCompressionFormat` is repeatedly called and the
exception is thrown everywhere, I think it's better to push the exception
thrown into the `getEffectiveCompressionFormat`.
For example:
```java
// DeepStorageSegmentConfig
public CompressionUtils.Format getEffectiveCompressionFormat(
final CompressionUtils.Format defaultFormat,
CompressionUtils.Format... supportedFormats
)
{
CompressionUtils.Format format = this.compressionFormat != null ?
this.compressionFormat : defaultFormat;
if (!Arrays.asList(supportedFormats).contains(format))
throw ...
}
return format;
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
