cshuo commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3911475873
##########
hudi-client/hudi-flink-client/src/main/java/org/apache/hudi/io/storage/row/HoodieRowDataFileWriterFactory.java:
##########
@@ -98,7 +99,10 @@ public HoodieFileWriter newParquetFileWriter(
boolean populateMetaFields =
MetaFieldsMode.resolve(config).toLegacyPopulateMetaFields();
boolean withOperation =
config.getBooleanOrDefault(HoodieWriteConfig.ALLOW_OPERATION_METADATA_FIELD);
- Pair<StorageConfiguration, HoodieConfig> injectedConfigs =
HoodieParquetConfigInjector.applyConfigInjector(storagePath, storage.getConf(),
config);
+ StorageConfiguration storageConf =
+ ParquetUtils.applyNativeLogZstdCompressionLevel(storagePath,
storage.getConf(), config);
Review Comment:
Fixed in 47c2d953e6e. Added ParquetUtils.prepareParquetWriterConfigs(...) to
apply the built-in native-log override followed by the custom injector, and
routed Spark, Flink, and Avro factories through it.
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/util/TestParquetUtils.java:
##########
@@ -90,6 +97,49 @@ public void setup() {
parquetUtils = new ParquetUtils();
}
+ @Test
+ void testApplyNativeLogZstdCompressionLevel() {
+ String parquetZstdLevel = "parquet.compression.codec.zstd.level";
Review Comment:
Fixed in 47c2d953e6e. The production code and test now use
ZstandardCodec.PARQUET_COMPRESS_ZSTD_LEVEL.
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/common/util/ParquetUtils.java:
##########
@@ -94,6 +97,32 @@
@Slf4j
public class ParquetUtils extends FileFormatUtils {
+ private static final String PARQUET_COMPRESSION_CODEC_ZSTD_LEVEL =
"parquet.compression.codec.zstd.level";
+
+ /**
+ * Returns a storage configuration with the native Parquet log ZSTD
compression level applied.
+ * The input configuration is copied only when its ZSTD level is absent or
differs from the native log level,
+ * so base file writers and other users of the shared configuration are not
affected.
+ */
+ public static <T> StorageConfiguration<T> applyNativeLogZstdCompressionLevel(
+ StoragePath path, StorageConfiguration<T> storageConf, HoodieConfig
hoodieConfig) {
+ if (!FSUtils.isNativeLogFile(path.getName())) {
+ return storageConf;
+ }
+
+ int nativeLogZstdLevel =
Review Comment:
Fixed in 47c2d953e6e. The code now uses
ZstandardCodec.PARQUET_COMPRESS_ZSTD_LEVEL. We intentionally retain fail-fast
validation for an explicitly malformed global value, with a clear
HoodieException and dedicated test, rather than silently hiding user
misconfiguration.
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##########
@@ -218,6 +218,14 @@ public class HoodieStorageConfig extends HoodieConfig {
.defaultValue("gzip")
Review Comment:
Fixed in 47c2d953e6e. The setting is now a ConfigProperty<Integer> with
defaultValue(1).
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/common/util/ParquetUtils.java:
##########
@@ -94,6 +97,32 @@
@Slf4j
public class ParquetUtils extends FileFormatUtils {
+ private static final String PARQUET_COMPRESSION_CODEC_ZSTD_LEVEL =
"parquet.compression.codec.zstd.level";
+
+ /**
+ * Returns a storage configuration with the native Parquet log ZSTD
compression level applied.
+ * The input configuration is copied only when its ZSTD level is absent or
differs from the native log level,
+ * so base file writers and other users of the shared configuration are not
affected.
+ */
+ public static <T> StorageConfiguration<T> applyNativeLogZstdCompressionLevel(
+ StoragePath path, StorageConfiguration<T> storageConf, HoodieConfig
hoodieConfig) {
Review Comment:
By design: the built-in native-log override runs before the custom injector
so an injector can still change the codec and/or override the level as the
highest-priority extension point. Gating before the injector would skip the
native level when an injector changes the codec to Zstd.
--
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]
