Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parquet: Revert workaround for resource usage with zstd #7834

Merged
merged 1 commit into from Jun 14, 2023

Conversation

bryanck
Copy link
Contributor

@bryanck bryanck commented Jun 14, 2023

This PR reverts #5681, which is no longer needed with the upgrade of Parquet to 1.13. This marks the now-unused codec factory class as deprecated.

Here is a heap leak analysis of one executor when running a large scan with a build of 1.2.1 (Parquet 1.12) and the workaround removed:
Screenshot 2023-06-13 at 5 04 54 PM

Here is the leak analysis with a build of this PR:
Screenshot 2023-06-13 at 5 04 26 PM

In the first example, the zstd buffer pool is accumulating memory. In the second example, there is no such accumulation.

Copy link
Contributor

@nastra nastra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @bryanck

@nastra nastra merged commit 4263135 into apache:master Jun 14, 2023
41 checks passed
nastra pushed a commit to nastra/iceberg that referenced this pull request Aug 15, 2023
* Spark: Update antlr4 to match Spark 3.4 (apache#7824)

* Parquet: Revert workaround for resource usage with zstd (apache#7834)

* GCP: fix single byte read in GCSInputStream (apache#8071)

* GCP: fix byte read in GCSInputStream

* add test

* Parquet: Cache codecs by name and level (apache#8182)

* GCP: Add prefix and bulk operations to GCSFileIO (apache#8168)

* AWS, GCS: Allow access to underlying storage client (apache#8208)

* spotless
rodmeneses pushed a commit to rodmeneses/iceberg that referenced this pull request Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants