You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "cshuo (via GitHub)" <gi...@apache.org> on 2026/08/28 09:50:54 UTC
[PR] feat(storage): support separate zstd level for native parquet logs [hudi]
cshuo opened a new pull request, #19781:
URL: https://github.com/apache/hudi/pull/19781
### Describe the issue this Pull Request addresses
Closes #19780.
The global Hadoop `parquet.compression.codec.zstd.level` setting currently applies to both Parquet base files and native Parquet log files. Native logs favor write latency and may not need the higher compression level selected for base files, but there is no independent native-log setting.
### Summary and Changelog
- Add the advanced `hoodie.logfile.parquet.compression.codec.zstd.level` configuration with a default value of `1`.
- Add a `ParquetUtils` helper that applies the native-log level only to native log paths, copying the storage configuration only when the global level is absent or different.
- Apply the built-in native-log override before `HoodieParquetConfigInjector` in Spark, Flink, and Avro/Java Parquet writer factories, preserving the custom injector as the highest-priority extension point.
- Add unit coverage for the default level, explicit override, equal-level configuration reuse, missing global configuration, and unaffected base-file paths.
### Impact
Users can tune native Parquet log Zstd compression independently from base files. Native logs default to level `1`, while base files retain the global Hadoop setting. This adds one optional advanced configuration and does not change Hudi's storage format or public APIs.
### Risk Level
Low. The override is limited to native log paths, base-file paths retain the original configuration instance, and custom Parquet config injectors still run last. The targeted `TestParquetUtils#testApplyNativeLogZstdCompressionLevel` and Flink Parquet writer factory test passed.
### Documentation Update
The new configuration and its default behavior are documented in `HoodieStorageConfig`. No storage-format documentation changes are required.
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 merged PR #19781:
URL: https://github.com/apache/hudi/pull/19781
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5505777090
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
}, {
"hash" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762",
"triggerID" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 937a22fcb02de96c528bb685c71e6ba607633f7e Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665)
* 47c2d953e6e762fb3ac3403c053ecf4eb99d6b06 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "cshuo (via GitHub)" <gi...@apache.org>.
cshuo commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3912741472
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/common/util/ParquetUtils.java:
##########
@@ -87,13 +92,58 @@
import static org.apache.parquet.avro.HoodieAvroParquetSchemaConverter.getAvroSchemaConverter;
import static org.apache.parquet.format.converter.ParquetMetadataConverter.NO_FILTER;
import static org.apache.parquet.format.converter.ParquetMetadataConverter.SKIP_ROW_GROUPS;
+import static org.apache.parquet.hadoop.codec.ZstandardCodec.PARQUET_COMPRESS_ZSTD_LEVEL;
/**
* Utility functions involving with parquet.
*/
@Slf4j
public class ParquetUtils extends FileFormatUtils {
+ /**
+ * Prepares the storage and Hudi configurations used by a Parquet writer. Built-in writer configuration
+ * overrides are applied first so that the user-provided config injector remains the highest-priority extension point.
+ */
+ public static Pair<StorageConfiguration, HoodieConfig> prepareParquetWriterConfigs(
Review Comment:
Fixed in 0d0fe351ea9. Renamed the helper to injectParquetWriterConfigs and updated all Spark, Flink, Avro, and test call sites.
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5519841334
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
}, {
"hash" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762",
"triggerID" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"triggerType" : "PUSH"
}, {
"hash" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16768",
"triggerID" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"triggerType" : "PUSH"
}, {
"hash" : "551998a7b845135005b4183e51a3ef100b4ba3a8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "551998a7b845135005b4183e51a3ef100b4ba3a8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0d0fe351ea92ba9cca96f5bb59e3980a3072086b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16768)
* 551998a7b845135005b4183e51a3ef100b4ba3a8 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5451678389
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/19781?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `82.60870%` with `4 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 78.11%. Comparing base ([`efe02e1`](https://app.codecov.io/gh/apache/hudi/commit/efe02e1621c17aa39a61716371a1857b4c086f79?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`937a22f`](https://app.codecov.io/gh/apache/hudi/commit/937a22fcb02de96c528bb685c71e6ba607633f7e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/19781?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...apache/hudi/common/config/HoodieStorageConfig.java](https://app.codecov.io/gh/apache/hudi/pull/19781?src=pr&el=tree&filepath=hudi-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fcommon%2Fconfig%2FHoodieStorageConfig.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2NvbmZpZy9Ib29kaWVTdG9yYWdlQ29uZmlnLmphdmE=) | 75.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/hudi/pull/19781?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...java/org/apache/hudi/common/util/ParquetUtils.java](https://app.codecov.io/gh/apache/hudi/pull/19781?src=pr&el=tree&filepath=hudi-hadoop-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fcommon%2Futil%2FParquetUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1oYWRvb3AtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2NvbW1vbi91dGlsL1BhcnF1ZXRVdGlscy5qYXZh) | 81.81% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/hudi/pull/19781?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19781 +/- ##
=========================================
Coverage 78.11% 78.11%
- Complexity 33673 33676 +3
=========================================
Files 2540 2540
Lines 141413 141433 +20
Branches 17123 17124 +1
=========================================
+ Hits 110467 110487 +20
- Misses 23250 23251 +1
+ Partials 7696 7695 -1
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hudi-common](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.57% <75.00%> (-0.01%)` | :arrow_down: |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.12% <ø> (+0.01%)` | :arrow_up: |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.66% <ø> (-0.01%)` | :arrow_down: |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.54% <ø> (ø)` | |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `74.50% <ø> (ø)` | |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `15.06% <ø> (ø)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.12% <86.66%> (+0.02%)` | :arrow_up: |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `75.63% <ø> (ø)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.76% <ø> (ø)` | |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.44% <ø> (ø)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.81% <ø> (ø)` | |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/19781/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.20% <ø> (ø)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/19781/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hadoop-mr-java-client](https://app.codecov.io/gh/apache/hudi/pull/19781/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `44.04% <82.60%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/19781?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...io/storage/row/HoodieRowDataFileWriterFactory.java](https://app.codecov.io/gh/apache/hudi/pull/19781?src=pr&el=tree&filepath=hudi-client%2Fhudi-flink-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fio%2Fstorage%2Frow%2FHoodieRowDataFileWriterFactory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jbGllbnQvaHVkaS1mbGluay1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW8vc3RvcmFnZS9yb3cvSG9vZGllUm93RGF0YUZpbGVXcml0ZXJGYWN0b3J5LmphdmE=) | `97.05% <ø> (-0.09%)` | :arrow_down: |
| [.../hudi/io/storage/HoodieSparkFileWriterFactory.java](https://app.codecov.io/gh/apache/hudi/pull/19781?src=pr&el=tree&filepath=hudi-client%2Fhudi-spark-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fio%2Fstorage%2FHoodieSparkFileWriterFactory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jbGllbnQvaHVkaS1zcGFyay1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW8vc3RvcmFnZS9Ib29kaWVTcGFya0ZpbGVXcml0ZXJGYWN0b3J5LmphdmE=) | `79.31% <ø> (-0.24%)` | :arrow_down: |
| [...io/storage/hadoop/HoodieAvroFileWriterFactory.java](https://app.codecov.io/gh/apache/hudi/pull/19781?src=pr&el=tree&filepath=hudi-hadoop-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fio%2Fstorage%2Fhadoop%2FHoodieAvroFileWriterFactory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1oYWRvb3AtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2lvL3N0b3JhZ2UvaGFkb29wL0hvb2RpZUF2cm9GaWxlV3JpdGVyRmFjdG9yeS5qYXZh) | `91.66% <100.00%> (+0.36%)` | :arrow_up: |
| [...apache/hudi/common/config/HoodieStorageConfig.java](https://app.codecov.io/gh/apache/hudi/pull/19781?src=pr&el=tree&filepath=hudi-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fcommon%2Fconfig%2FHoodieStorageConfig.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2NvbmZpZy9Ib29kaWVTdG9yYWdlQ29uZmlnLmphdmE=) | `89.40% <75.00%> (-0.33%)` | :arrow_down: |
| [...java/org/apache/hudi/common/util/ParquetUtils.java](https://app.codecov.io/gh/apache/hudi/pull/19781?src=pr&el=tree&filepath=hudi-hadoop-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fcommon%2Futil%2FParquetUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1oYWRvb3AtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2NvbW1vbi91dGlsL1BhcnF1ZXRVdGlscy5qYXZh) | `90.90% <81.81%> (-0.51%)` | :arrow_down: |
... and [2 files with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/19781/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5507601990
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
}, {
"hash" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762",
"triggerID" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"triggerType" : "PUSH"
}, {
"hash" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 47c2d953e6e762fb3ac3403c053ecf4eb99d6b06 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762)
* 0d0fe351ea92ba9cca96f5bb59e3980a3072086b UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3920902625
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/common/util/ParquetUtils.java:
##########
@@ -94,6 +97,17 @@
@Slf4j
public class ParquetUtils extends FileFormatUtils {
+ /**
+ * Applies built-in injectors followed by the user-defined injector to the configurations used by a Parquet writer.
+ */
+ public static Pair<StorageConfiguration, HoodieConfig> injectParquetWriterConfigs(
Review Comment:
just put the logic inside HoodieParquetConfigInjector.applyConfigInjector
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5520705434
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
}, {
"hash" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762",
"triggerID" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"triggerType" : "PUSH"
}, {
"hash" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16768",
"triggerID" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"triggerType" : "PUSH"
}, {
"hash" : "551998a7b845135005b4183e51a3ef100b4ba3a8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16786",
"triggerID" : "551998a7b845135005b4183e51a3ef100b4ba3a8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 551998a7b845135005b4183e51a3ef100b4ba3a8 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16786)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5505710127
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
}, {
"hash" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 937a22fcb02de96c528bb685c71e6ba607633f7e Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665)
* 47c2d953e6e762fb3ac3403c053ecf4eb99d6b06 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5508899022
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
}, {
"hash" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762",
"triggerID" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"triggerType" : "PUSH"
}, {
"hash" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16768",
"triggerID" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0d0fe351ea92ba9cca96f5bb59e3980a3072086b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16768)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "cshuo (via GitHub)" <gi...@apache.org>.
cshuo commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3911473621
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##########
@@ -218,6 +218,14 @@ public class HoodieStorageConfig extends HoodieConfig {
.defaultValue("gzip")
.withDocumentation("Compression Codec for parquet files");
+ public static final ConfigProperty<String> LOGFILE_PARQUET_COMPRESSION_CODEC_ZSTD_LEVEL = ConfigProperty
+ .key("hoodie.logfile.parquet.compression.codec.zstd.level")
+ .defaultValue("1")
Review Comment:
By design: native Parquet log Zstd level intentionally defaults to 1 and overrides the global setting, while base files continue to use the global level. This behavior is documented in the config and PR impact section.
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5519989066
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
}, {
"hash" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762",
"triggerID" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"triggerType" : "PUSH"
}, {
"hash" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16768",
"triggerID" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"triggerType" : "PUSH"
}, {
"hash" : "551998a7b845135005b4183e51a3ef100b4ba3a8",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16786",
"triggerID" : "551998a7b845135005b4183e51a3ef100b4ba3a8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0d0fe351ea92ba9cca96f5bb59e3980a3072086b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16768)
* 551998a7b845135005b4183e51a3ef100b4ba3a8 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16786)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "cshuo (via GitHub)" <gi...@apache.org>.
cshuo commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3920941995
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/common/util/ParquetUtils.java:
##########
@@ -94,6 +97,17 @@
@Slf4j
public class ParquetUtils extends FileFormatUtils {
+ /**
+ * Applies built-in injectors followed by the user-defined injector to the configurations used by a Parquet writer.
+ */
+ public static Pair<StorageConfiguration, HoodieConfig> injectParquetWriterConfigs(
Review Comment:
HoodieParquetConfigInjector is in hudi-common, while ParquetZstdCompressionLevelInjector needs to stay in hudi-hadoop-common because it depends on Parquet/Hadoop classes, like `ZstandardCodec`.
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5451131759
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 937a22fcb02de96c528bb685c71e6ba607633f7e UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3901121902
##########
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:
🤖 nit: this string literal duplicates the private `PARQUET_COMPRESSION_CODEC_ZSTD_LEVEL` constant in `ParquetUtils`. If the constant changes, the test would silently pass against the wrong key. Could you either make the constant package-private (or `@VisibleForTesting`) so the test can reference it directly, or at least add a comment tying it back to the source constant?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
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:
🤖 This applies the zstd level to every native log write regardless of the actual log compression codec (default is `gzip`). When the codec isn't zstd, setting `parquet.compression.codec.zstd.level` is a no-op but still forces a full `Configuration` deep-copy per log-writer creation. Since `hoodieConfig` already carries `PARQUET_COMPRESSION_CODEC_NAME`, would it make sense to gate this on the codec actually being zstd?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##########
@@ -218,6 +218,14 @@ public class HoodieStorageConfig extends HoodieConfig {
.defaultValue("gzip")
.withDocumentation("Compression Codec for parquet files");
+ public static final ConfigProperty<String> LOGFILE_PARQUET_COMPRESSION_CODEC_ZSTD_LEVEL = ConfigProperty
+ .key("hoodie.logfile.parquet.compression.codec.zstd.level")
+ .defaultValue("1")
Review Comment:
🤖 Since this defaults to `1` and `applyNativeLogZstdCompressionLevel` applies it whenever it differs from the global setting, existing tables using zstd for native log files will silently drop to level 1 on upgrade (from their configured global `parquet.compression.codec.zstd.level`, or parquet's default of 3 if unset). Is that intended for existing users, or should the override only kick in when this config is explicitly set and otherwise inherit the global level? Might be worth calling out as a behavior change either way.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3910843806
##########
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:
Two small robustness improvements here: can we use Parquet's `ZstandardCodec.PARQUET_COMPRESS_ZSTD_LEVEL` constant instead of duplicating the key (or document why an older-Parquet compatibility constraint requires the literal), and compare the existing value with the desired value as strings instead of calling `Integer.parseInt`? A malformed inherited global value should be replaceable by an explicit native-log override rather than failing before the override is applied.
##########
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:
Could this be a `ConfigProperty<Integer>` with `defaultValue(1)`? The setting is numeric and is consumed through `getIntOrDefault`; using the typed form would match Hudi's other numeric configs and make the contract clearer.
##########
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:
Could we compose this built-in native-log override and the custom injector behind one shared `prepareParquetWriterConfigs(...)` helper? The same two-step setup is now repeated in the Flink, Spark, and Avro factories, so a future Parquet writer path could easily miss it. Keeping the custom injector last would preserve its ability to override the built-in setting.
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3912563203
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/common/util/ParquetUtils.java:
##########
@@ -87,13 +92,58 @@
import static org.apache.parquet.avro.HoodieAvroParquetSchemaConverter.getAvroSchemaConverter;
import static org.apache.parquet.format.converter.ParquetMetadataConverter.NO_FILTER;
import static org.apache.parquet.format.converter.ParquetMetadataConverter.SKIP_ROW_GROUPS;
+import static org.apache.parquet.hadoop.codec.ZstandardCodec.PARQUET_COMPRESS_ZSTD_LEVEL;
/**
* Utility functions involving with parquet.
*/
@Slf4j
public class ParquetUtils extends FileFormatUtils {
+ /**
+ * Prepares the storage and Hudi configurations used by a Parquet writer. Built-in writer configuration
+ * overrides are applied first so that the user-provided config injector remains the highest-priority extension point.
+ */
+ public static Pair<StorageConfiguration, HoodieConfig> prepareParquetWriterConfigs(
Review Comment:
-> ingectParquetWriterConfigs
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5506828374
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
}, {
"hash" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762",
"triggerID" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 47c2d953e6e762fb3ac3403c053ecf4eb99d6b06 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5452200677
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 937a22fcb02de96c528bb685c71e6ba607633f7e Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3920758722
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/core/io/ParquetZstdCompressionLevelInjector.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.core.io;
+
+import org.apache.hudi.common.config.HoodieConfig;
+import org.apache.hudi.common.config.HoodieStorageConfig;
+import org.apache.hudi.common.fs.FSUtils;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.storage.StorageConfiguration;
+import org.apache.hudi.storage.StoragePath;
+
+import static org.apache.parquet.hadoop.codec.ZstandardCodec.PARQUET_COMPRESS_ZSTD_LEVEL;
+
+/**
+ * Built-in injector that configures the ZSTD compression level for native Parquet log files.
+ */
+public class ParquetZstdCompressionLevelInjector implements HoodieParquetConfigInjector {
+
+ public static ParquetZstdCompressionLevelInjector INSTANCE = new ParquetZstdCompressionLevelInjector();
Review Comment:
🤖 nit: could you make this singleton `final`? A public mutable static field could be reassigned by accident.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3912569450
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/common/util/ParquetUtils.java:
##########
@@ -87,13 +92,58 @@
import static org.apache.parquet.avro.HoodieAvroParquetSchemaConverter.getAvroSchemaConverter;
import static org.apache.parquet.format.converter.ParquetMetadataConverter.NO_FILTER;
import static org.apache.parquet.format.converter.ParquetMetadataConverter.SKIP_ROW_GROUPS;
+import static org.apache.parquet.hadoop.codec.ZstandardCodec.PARQUET_COMPRESS_ZSTD_LEVEL;
/**
* Utility functions involving with parquet.
*/
@Slf4j
public class ParquetUtils extends FileFormatUtils {
+ /**
+ * Prepares the storage and Hudi configurations used by a Parquet writer. Built-in writer configuration
+ * overrides are applied first so that the user-provided config injector remains the highest-priority extension point.
+ */
+ public static Pair<StorageConfiguration, HoodieConfig> prepareParquetWriterConfigs(
+ StoragePath path, StorageConfiguration storageConf, HoodieConfig hoodieConfig) {
+ StorageConfiguration nativeLogStorageConf =
+ applyNativeLogZstdCompressionLevel(path, storageConf, hoodieConfig);
+ return HoodieParquetConfigInjector.applyConfigInjector(path, nativeLogStorageConf, hoodieConfig);
+ }
+
+ /**
+ * 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(
Review Comment:
-> ingectDefaultZstdCompressionLevel
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5507674840
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
}, {
"hash" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762",
"triggerID" : "47c2d953e6e762fb3ac3403c053ecf4eb99d6b06",
"triggerType" : "PUSH"
}, {
"hash" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16768",
"triggerID" : "0d0fe351ea92ba9cca96f5bb59e3980a3072086b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 47c2d953e6e762fb3ac3403c053ecf4eb99d6b06 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16762)
* 0d0fe351ea92ba9cca96f5bb59e3980a3072086b Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16768)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "cshuo (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "cshuo (via GitHub)" <gi...@apache.org>.
cshuo commented on code in PR #19781:
URL: https://github.com/apache/hudi/pull/19781#discussion_r3912743969
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/common/util/ParquetUtils.java:
##########
@@ -87,13 +92,58 @@
import static org.apache.parquet.avro.HoodieAvroParquetSchemaConverter.getAvroSchemaConverter;
import static org.apache.parquet.format.converter.ParquetMetadataConverter.NO_FILTER;
import static org.apache.parquet.format.converter.ParquetMetadataConverter.SKIP_ROW_GROUPS;
+import static org.apache.parquet.hadoop.codec.ZstandardCodec.PARQUET_COMPRESS_ZSTD_LEVEL;
/**
* Utility functions involving with parquet.
*/
@Slf4j
public class ParquetUtils extends FileFormatUtils {
+ /**
+ * Prepares the storage and Hudi configurations used by a Parquet writer. Built-in writer configuration
+ * overrides are applied first so that the user-provided config injector remains the highest-priority extension point.
+ */
+ public static Pair<StorageConfiguration, HoodieConfig> prepareParquetWriterConfigs(
+ StoragePath path, StorageConfiguration storageConf, HoodieConfig hoodieConfig) {
+ StorageConfiguration nativeLogStorageConf =
+ applyNativeLogZstdCompressionLevel(path, storageConf, hoodieConfig);
+ return HoodieParquetConfigInjector.applyConfigInjector(path, nativeLogStorageConf, hoodieConfig);
+ }
+
+ /**
+ * 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(
Review Comment:
Fixed in 0d0fe351ea9. Renamed the helper to injectDefaultZstdCompressionLevel and updated the related tests and documentation.
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): support separate zstd level for native parquet logs [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19781:
URL: https://github.com/apache/hudi/pull/19781#issuecomment-5451311745
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665",
"triggerID" : "937a22fcb02de96c528bb685c71e6ba607633f7e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 937a22fcb02de96c528bb685c71e6ba607633f7e Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16665)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org