Skip to content

KAFKA-12442: Upgrade ZSTD JNI from 1.4.8-4 to 1.4.9-1#10285

Merged
chia7712 merged 1 commit intoapache:trunkfrom
dongjoon-hyun:KAFKA-12442
Mar 12, 2021
Merged

KAFKA-12442: Upgrade ZSTD JNI from 1.4.8-4 to 1.4.9-1#10285
chia7712 merged 1 commit intoapache:trunkfrom
dongjoon-hyun:KAFKA-12442

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Mar 9, 2021

This PR aims to upgrade ZSTD JNI from 1.4.8-4 to 1.4.9-1.

`ZStandard 1.4.9 and its corresponding JNI brings the following bug fixes and improvements.

One of notable improvement of ZStandard 1.4.9 is 2x faster Long Distance Mode, but we are not using it yet.

Since this is a dependency change, this should pass all the existing CIs.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@dongjinleekr
Copy link
Contributor

@dongjoon-hyun Hi Hyun, it looks good to me. I love this update. 😄

Copy link
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@dongjoon-hyun thanks for your patch. No obvious performance regression from performance test (benchmark_test.py). +1

@dongjoon-hyun
Copy link
Member Author

Thank you, @chia7712 and @dongjinleekr .

@dongjoon-hyun
Copy link
Member Author

Hi, @ijuma . Could you review this PR please?

Copy link
Member

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

LGTM. To be clear, there is no benefit from this change right now?

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Mar 11, 2021

Since the new features are not used, you may right. However, I believe the benefits are three-fold.

  • There exists ZSTD side bug-fixes at ZSTD 1.4.9.
  • There exists ZSTD JNI side memory optimization improvements at ZSTD JNI 1.4.8-5 ~ 1.4.8-7. (This includes some incompatible changes and recovery. So, 1.4.9 is more human-readable stable version number.).
  • I hope this will reduce the chance of future potential version conflict issues across Apache projects. It's important when some downstream project starts to use new feature.

@ijuma
Copy link
Member

ijuma commented Mar 11, 2021

Sounds good.

@dongjoon-hyun
Copy link
Member Author

Thank you, @ijuma .

@chia7712 chia7712 merged commit 1f9c9f8 into apache:trunk Mar 12, 2021
@dongjoon-hyun
Copy link
Member Author

Thank you for review and merging, @chia7712 and all!

@dongjoon-hyun dongjoon-hyun deleted the KAFKA-12442 branch March 12, 2021 05:43
@ijuma
Copy link
Member

ijuma commented Mar 12, 2021

@chia7712 worth cherry-picking to 2.8 if the aim is to align the versions with other projects that are releasing soon.

@chia7712
Copy link
Member

worth cherry-picking to 2.8 if the aim is to align the versions with other projects that are releasing soon.

done!

chia7712 pushed a commit that referenced this pull request Mar 13, 2021
Since the new features are not used, you may right. However, I believe the benefits are three-fold.
- There exists ZSTD side bug-fixes at ZSTD 1.4.9.
- There exists ZSTD JNI side memory optimization improvements at ZSTD JNI 1.4.8-5 ~ 1.4.8-7. (This includes some incompatible changes and recovery. So, 1.4.9 is more human-readable stable version number.).
- I hope this will reduce the chance of future potential version conflict issues across Apache projects. It's important when some downstream project starts to use new feature.
   - Apache Spark 3.2.0 will use ZSTD 1.4.9. (apache/spark@ba7e525)
   - Apache Parquet 1.12.0 will use ZSTD 1.4.9 (apache/parquet-java@66ac28c)
   - Apache Avro 1.10.3 will use ZSTD 1.4.9 (apache/avro@806667c)

Reviewers: Lee Dongjin <dongjin@apache.org>, Ismael Juma <ismael@juma.me.uk>, Chia-Ping Tsai <chia7712@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants