PARQUET-1973: Support ZSTD JNI BufferPool

[dongjoon-hyun]

Jira

• My PR addresses the following Parquet Jira issues and references them in the PR title. For example, "PARQUET-1234: My Parquet PR"
  • https://issues.apache.org/jira/browse/PARQUET-1973
  • This PR aims to add parquet.compression.codec.zstd.bufferPool.enabled to support ZSTD JNI's BufferPool feature.

Version	Description	Commit
v1.4.5-7	BufferPool was added and used it by default	luben/zstd-jni@4f55c89
v1.4.5-8	RecyclingBufferPool was added and BufferPool became an interface to allow custom BufferPool implementation	luben/zstd-jni@dd2588e
v1.4.7+	NoPool is used by default and user should specify buffer pool explicitly	luben/zstd-jni@f7c8279

Tests

• My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

• My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Documentation

• In case of new functionality, my PR adds documentation that describes how to use it.
  • All the public functions and the classes in the PR contain Javadoc that explain what it does

[gszadovszky]

What would be the pros/cons for a user to use or to not use the buffer pool in parquet-mr? I understand that you want to be on the safe side with this new configuration property but I am currently not sure if it is necessary. I would not extend the number of the existing configuration options if it is not required.

Anyway, if you think the configuration option is necessary, please, update the documentation in the README.md.

[dongjoon-hyun]

Hi, @gszadovszky . We observed this kind of issue on Java 11 environment.

• Detected Native memory leak or slow reclamation of native memory luben/zstd-jni#156

As I wrote in the PR description, NoPool is the default of ZSTD JNI because Using soft/weak references could be harmful on some workloads. (luben/zstd-jni@f7c8279)

[dongjoon-hyun]

Not only for the issues, but also there exists improvement. FYI, Apache Spark is using ZSTD JNI directly for shuffle IO and the followings are the corresponding update and benchmark PRs.

• [SPARK-34340][CORE] Support ZSTD JNI BufferPool spark#31453 (SPARK-34340 Support ZSTD JNI BufferPool)
• [SPARK-34387][CORE][TESTS] Add ZStandardBenchmark spark#31498 (SPARK-34387 Add ZStandardBenchmark)

[gszadovszky]

Thanks for explaining. I've found a formatting issue in the readme. Otherwise it looks good to me.

[gszadovszky]

nit: please use double spaces at the end of the lines to enforce new-line. You can verify formatting by checking the page rendered by github: https://github.com/dongjoon-hyun/parquet-mr/tree/PARQUET-1973/parquet-hadoop#class-zstandardcodec

[dongjoon-hyun]

Oh. Got it. Thanks!

[dongjoon-hyun]

Thank you, @gszadovszky . I added two ending spaces and checked the README.

[dongjoon-hyun]

Thank you for approval, @gszadovszky .

[dongjoon-hyun]

Thank you for merging, @gszadovszky !

[shangxinli]

Thanks Dongjoon for working on this!

It is kind of late. Just a minor comment: if you can wrap the code into a method and call it in both CompressionInputStream() and CompressionOutputStream, it would avoid duplicating. Not a big deal though.

[shangxinli]

cc @vectorijk