-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[fix][ci] Fix snappy-java native lib fails to load in x86 alpine #22804
[fix][ci] Fix snappy-java native lib fails to load in x86 alpine #22804
Conversation
I verified this PR and it works fine.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #22804 +/- ##
============================================
- Coverage 73.57% 73.22% -0.35%
+ Complexity 32624 32617 -7
============================================
Files 1877 1890 +13
Lines 139502 141644 +2142
Branches 15299 15541 +242
============================================
+ Hits 102638 103721 +1083
- Misses 28908 29917 +1009
- Partials 7956 8006 +50
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left some minor comments. Good work @yaalsn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please find another solution than modifying the default bin/pulsar
script
We don't need to add that in the OPTS by default. We can add a comment to say if you need snappy, add this |
Nice! This way is better. |
I don't think that this is a good idea to have more configuration options to tweak. It's better to handle this in the Dockerfile building phase as I suggested by simply appending |
I also prefer this solution. |
both ok to me. |
One problem is that building snappy may take a lot of time. I haven't tested it yet. |
It doesn't take long to build. |
@lhotari PTAL |
) (cherry picked from commit 7b8f4a9)
Motivation
From branch-3.3, pulsar starts to use alpine as base image, but it throws exception when using snappy-java lib.
The reason is that alpine is musl-based OS while snappy-java lib built-in libsnappyjava.so is compiled with glibc.
In order to fix this issue, we need to re-compile the snappy-java native lib in alpine docker.
Modifications
Verifying this change
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: yaalsn#2