FrankChen021 commented on code in PR #20806:
URL: https://github.com/apache/druid/pull/20806#discussion_r4434129627
##########
processing/src/main/java/org/apache/druid/segment/loading/DeepStorageSegmentConfig.java:
##########
@@ -41,33 +44,105 @@
public class DeepStorageSegmentConfig
{
@JsonProperty
+ @Deprecated
Review Comment:
I think we can remove this zip property from this class. in the ctor which
is annotated with JsonCreator, we can compute the compressionFormat is zip
property is used.
##########
processing/src/main/java/org/apache/druid/segment/loading/DeepStorageSegmentConfig.java:
##########
@@ -41,33 +44,105 @@
public class DeepStorageSegmentConfig
{
@JsonProperty
+ @Deprecated
@Nullable
private final Boolean zip;
+ @JsonProperty
+ @Nullable
+ private final CompressionUtils.Format compressionFormat;
+
@JsonCreator
- public DeepStorageSegmentConfig(@JsonProperty("zip") @Nullable Boolean zip)
+ public DeepStorageSegmentConfig(
+ @JsonProperty("zip") @Nullable Boolean zip,
+ @JsonProperty("compressionFormat") @Nullable CompressionUtils.Format
compressionFormat
+ )
{
+ validate(zip, compressionFormat);
this.zip = zip;
+ this.compressionFormat = compressionFormat;
+ }
+
+ public DeepStorageSegmentConfig(@Nullable Boolean zip)
+ {
+ this(zip, null);
}
public DeepStorageSegmentConfig()
{
- this(null);
+ this(null, null);
+ }
+
+ private static void validate(@Nullable Boolean zip, @Nullable
CompressionUtils.Format compressionFormat)
+ {
+ if (zip == null || compressionFormat == null) {
+ return;
+ }
+ if (compressionFormat == CompressionUtils.Format.ZIP && !zip) {
+ throw new IAE(
+ "Conflicting compression configuration:
druid.storage.compressionFormat is [%s] but druid.storage.zip is [%s]",
+ compressionFormat,
+ zip
+ );
+ }
+ if (compressionFormat == CompressionUtils.Format.NONE && zip) {
+ throw new IAE(
+ "Conflicting compression configuration:
druid.storage.compressionFormat is [%s] but druid.storage.zip is [%s]",
+ compressionFormat,
+ zip
+ );
+ }
+ if (compressionFormat != CompressionUtils.Format.ZIP && compressionFormat
!= CompressionUtils.Format.NONE) {
+ throw new IAE(
+ "Conflicting compression configuration: druid.storage.zip cannot be
configured when druid.storage.compressionFormat is [%s]",
+ compressionFormat
+ );
+ }
Review Comment:
when zip is not null, let's log a warning message to say it's deprecated and
suggest the use of compressionFormat
##########
docs/configuration/index.md:
##########
@@ -432,7 +432,8 @@ This deep storage is used to interface with HDFS. You must
load the `druid-hdfs-
|Property|Description|Default|
|--------|-----------|-------|
|`druid.storage.storageDirectory`|HDFS directory to use as deep storage.|none|
-|`druid.storage.compressionFormat`|The compression format applied to the
segments uploaded to HDFS. Only `zip` and `lz4` are supported. |zip|
+|`druid.storage.compressionFormat`|The compression format applied to segments
uploaded to HDFS. Supports `zip`, `lz4`, and `none` (uncompressed directory
layout).|zip|
+|`druid.storage.zip`|Deprecated. Use `druid.storage.compressionFormat`
instead. Whether segments uploaded to HDFS are written as directories (`false`)
or zip files (`true`).|true|
Review Comment:
no need to mention `druid.storage.zip` here
--
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]
