You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/06/03 01:20:14 UTC
[GitHub] [iceberg] kbendick opened a new pull request, #4951: Core - Upgrade Parquet to 1.12.3 to get Zstd Buffer Pool by default
kbendick opened a new pull request, #4951:
URL: https://github.com/apache/iceberg/pull/4951
Some users have noticed occasional OOMs due to memory fragmentation when using ZSTD.
There's a configuration property `parquet.compression.codec.zstd.bufferPool.enabled`, which is defaulted to `false` in parquet-mr <= 1.12.2. As of 1.12.3, it's defaulted to true.
Given that we know the zstd OOMs are related to memory fragmentation, likely the updated configuration value might help resolve this.
Given it's one minor patch version away, I'm opening this draft PR to upgrade the version while I investigate this issue.
If anybody has anytime to try and test with this setting, that would be great!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4951: Core - Upgrade Parquet to 1.12.3
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1170433413
> @kbendick, what's the status of this? Should Iceberg set `parquet.compression.codec.zstd.bufferPool.enabled=true` for all Parquet files? That sounds like a good thing to me.
I have spoken to some users with very high cardinality tables (potentially over 1000 columns) who have told me that enabling the buffer pool via this configuration has resolved OOMs for them, so I believe it will be beneficial to all users.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4951: [TEST] Core - Upgrade Parquet to 1.12.3 to get Zstd Buffer Pool by default
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1170298698
> @kbendick, what's the status of this? Should Iceberg set `parquet.compression.codec.zstd.bufferPool.enabled=true` for all Parquet files? That sounds like a good thing to me.
Yes I believe we should set that configuration by default. Either by upgrading the patch version of parquet, or setting it ourselves.
If we choose to set it ourselves and _not_ upgrade the parquet library, I'll do a quick pass over the PRs added between 1.12.2 and 1.12.3 to make sure it's not somehow unsafe to add.
I think upgrading the parquet patch version would likely be better in the longer term.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4951: Core - Upgrade Parquet to 1.12.3
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1170458186
The Hive tests are failing due to the lack of a method.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #4951: [TEST] Core - Upgrade Parquet to 1.12.3 to get Zstd Buffer Pool by default
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#discussion_r910263797
##########
versions.props:
##########
@@ -6,7 +6,7 @@ org.apache.hadoop:* = 2.7.3
org.apache.hive:* = 2.3.8
org.apache.httpcomponents.client5:* = 5.1
org.apache.orc:* = 1.7.5
-org.apache.parquet:* = 1.12.2
+org.apache.parquet:* = 1.12.3
Review Comment:
Yeah absolutely.
This PR was originally for running the JMH tests.
I’ll remove all changes but the version bump and consider opening a separate PR for them if we want them.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4951: Core - Upgrade Parquet to 1.12.3
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1175744382
Closed in favor of https://github.com/apache/iceberg/pull/5188, which gets this working.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4951: [TEST] Core - Upgrade Parquet to 1.12.3 to get Zstd Buffer Pool by default
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1170311953
For reference, [1.12.3 is the default parquet version in Spark master](https://github.com/apache/spark/blob/c0a12cfbb82a710d786158bcd70a7f7bd531751b/pom.xml#L134), although [Spark 3.3.0 still uses Parquet 1.12.2](https://github.com/apache/spark/blob/f74867bddfbcdd4d08076db36851e88b15e66556/pom.xml#L134).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #4951: [TEST] Core - Upgrade Parquet to 1.12.3 to get Zstd Buffer Pool by default
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#discussion_r910260232
##########
versions.props:
##########
@@ -6,7 +6,7 @@ org.apache.hadoop:* = 2.7.3
org.apache.hive:* = 2.3.8
org.apache.httpcomponents.client5:* = 5.1
org.apache.orc:* = 1.7.5
-org.apache.parquet:* = 1.12.2
+org.apache.parquet:* = 1.12.3
Review Comment:
Looks like this is the only necessary change. Can you update this PR to just this change, or can you open a separate PR for this?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick closed pull request #4951: Core - Upgrade Parquet to 1.12.3
Posted by GitBox <gi...@apache.org>.
kbendick closed pull request #4951: Core - Upgrade Parquet to 1.12.3
URL: https://github.com/apache/iceberg/pull/4951
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4951: [TEST] Core - Upgrade Parquet to 1.12.3 to get Zstd Buffer Pool by default
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1170327011
We might also want to bump the Avro version to match what’s used in parquet 1.12.3.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #4951: [TEST] Core - Upgrade Parquet to 1.12.3 to get Zstd Buffer Pool by default
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1170356941
> We might also want to bump the Avro version to match what’s used in parquet 1.12.3.
Is that safe? What is the version change?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4951: Core - Upgrade Parquet to 1.12.3
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1171408583
> Looks like Hive might be bringing in a different copy of Parquet and that is conflicting in the test. We should be able to exclude Hive's Parquet version to work around this.
I'll give that a try.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #4951: [TEST] Core - Upgrade Parquet to 1.12.3 to get Zstd Buffer Pool by default
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1170246398
@kbendick, what's the status of this? Should Iceberg set `parquet.compression.codec.zstd.bufferPool.enabled=true` for all Parquet files? That sounds like a good thing to me.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #4951: [TEST] Core - Upgrade Parquet to 1.12.3 to get Zstd Buffer Pool by default
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#discussion_r910259548
##########
spark/v3.2/spark/src/jmh/java/org/apache/iceberg/spark/source/IcebergSourceFlatDataBenchmark.java:
##########
@@ -54,6 +54,8 @@ protected final Table initTable() {
HadoopTables tables = new HadoopTables(hadoopConf());
Map<String, String> properties = Maps.newHashMap();
properties.put(TableProperties.METADATA_COMPRESSION, "gzip");
+ // Force usage of zstd compression for all Iceberg Source Flat Data Benchmarks that use Parquet
+ properties.put(TableProperties.PARQUET_COMPRESSION, "zstd");
Review Comment:
Can you remove the unnecessary changes like this? If you want to do this, then let's move it to a separate PR.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4951: Core - Upgrade Parquet to 1.12.3
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1170430464
> > We might also want to bump the Avro version to match what’s used in parquet 1.12.3.
>
> Is that safe? What is the version change?
The version change is from 1.10.1 to 1.10.2. https://github.com/apache/parquet-mr/commit/d96b19bb97caf6f358579c9e22626553e8dc986d
We are on 1.10.1, so we would be following the same upgrade path.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #4951: Core - Upgrade Parquet to 1.12.3
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #4951:
URL: https://github.com/apache/iceberg/pull/4951#issuecomment-1170568977
Looks like Hive might be bringing in a different copy of Parquet and that is conflicting in the test. We should be able to exclude Hive's Parquet version to work around this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org