phoenix PHOENIX-7482 Replace uses of org.iq80.snappy:snappy with org.xerial.snappy:snappy-java

Dec 09 '24 21:12 apurtell

Pushed an update, see 24963c3 Xerial snappy cannot be shaded because there is a JNI component. Shading can't/won't rewrite the native symbols. The result is runtime link failures. We ran a local precommit at $dayjob and it passed, this issue was found later, so there may not be test coverage for this case.

Dec 19 '24 20:12 apurtell

It seems that Hadoop also depends on this library.

I wonder if that could cause JNI version conflict problems when running with external Hadoop ? I haven't dug into this, just a thought. (Actually, I'm not sure how we handle native dependencies when using external Hadoop at all)

Jan 09 '25 07:01 stoty

The precommit test links are stale. Let me rebase this on the latest master and push the update to trigger another round. I did a local RAT check and it passed.

Jan 13 '25 21:01 apurtell

@stoty

I wonder if that could cause JNI version conflict problems when running with external Hadoop ?

The native code is shipped as a resource inside the jar and is dynamically loaded from the jar by the Java code also shipped in that jar. It is triggered by the Snappy class static initializer and calls SnappyLoader which will load the native libraries with class level synchronization. So this will be done only once per JVM, in a way that prevents version mismatch, no matter if even multiple versions of the xerial snappy jar are on the classpath.

Jan 13 '25 21:01 apurtell

Ok, of the reported issues, the ASF license problems are not related to this patch:

phoenix-client-parent/target/javadoc-bundle-options/javadoc-options-javadoc-resources.xml:1:Missing Apache License
phoenix-server/target/javadoc-bundle-options/javadoc-options-javadoc-resources.xml:1:Missing Apache License
phoenix-mapreduce-byo-shaded-hbase/target/javadoc-bundle-options/javadoc-options-javadoc-resources.xml:1:Missing Apache License

I've pushed a fix for the checkstyle findings. Line length issues.

WALRecoveryRegionPostOpenIT seems flaky, in that it usually passes for me locally both on current master and master plus this change, and also sometimes it doesn't pass, and that does not seem tied to the change either.

Jan 14 '25 19:01 apurtell

@stoty @apurtell are we good to merge this?

Apr 12 '25 05:04 virajjasani

Sure, I thought this has been merged some time ago.

Apr 14 '25 04:04 stoty

Triggered a new build: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-2039/5/

Apr 14 '25 06:04 virajjasani

I think we are good on the change, thanks for kicking this. Let's check the next precommit results...

Apr 14 '25 15:04 apurtell

The latest result looks good: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-2039/6/PR_20General_20Check_20Report/

Two test failures are known flakies.

Apr 15 '25 04:04 virajjasani