You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/06/14 13:38:46 UTC
[GitHub] [flink-docker] nferrario opened a new pull request, #117: Fix LD_PRELOAD on ARM images
nferrario opened a new pull request, #117:
URL: https://github.com/apache/flink-docker/pull/117
This fixes an error that prevents jemalloc from being loaded on ARM images:
```
ERROR: ld.so: object '/usr/lib/x86_64-linux-gnu/libjemalloc.so' from LD_PRELOAD cannot be preloaded (cannot open shared object file): ignored.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r928771807
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
@nferrario We can adjust the test itself instead of just disabling the test, e.g. we can directly `exit 0` if the expected jemalloc binary so file does not exist.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r917815786
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
Because LD_PRELOAD used to have a value even when jemalloc didn't exist in the host. Now LD_PRELOAD will be empty on that scenario, and since the test suite isn't running on a Flink Docker image, but rather on just plain Github (or a local machine), we only have glibc available
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r916737952
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
@Myasuka Hmm, the tests pass if I run `bash testing/run_tests.sh` instead of `./testing/run_tests.sh`. Is it ok if I change `ci.yml` to:
```yaml
- name: "Test images"
run: bash testing/run_tests.sh
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r916719544
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
Considering we already get the error I posted in the first PR comment as the very first line of the Job Manager and Task Manager docker logs, Idk how much more it would help to add more warnings. It already is a very low level thing that you won't be able to notice unless you specifically look for it, because there's no way to know that Jemalloc isn't being used (it took months for us to realize).
I will however add a message that's easier to understand, since the default LD_PRELOAD error still requires the user to look at the source code or the Docker image structure at the very least.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r916746986
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
@Myasuka I don't think there's a good way to have the tests actually check if LD_PRELOAD is set. I'm not sure if you'd want me to `touch`, which might be fine on Github Actions but might be annoying when testing locally, or just ignore the test entirely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on PR #117:
URL: https://github.com/apache/flink-docker/pull/117#issuecomment-1156015366
@nferrario actually, you should make this PR target for `dev-master` branch first, and we can cherry-pick the final commit to `dev-1.15` after code review if no obvious code conflict.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r916413243
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
Based on current comments, we can use `uname -m` to get the machine hardware name, and check whether the file existed. If not, we fall back to default location `/usr/lib/x86_64-linux-gnu/libjemalloc.so`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r916647774
##########
docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
You're absolutely right, it is `-m` indeed. Fixing now. Thank you all for your input
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r927645868
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
I agree, but the test will never work unless we run a Dockerized test. We cannot fake a system library in a way that works for multiple OS (Linux, macOS)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r916651193
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
@nferrario did you ever consider this suggestion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] zentol commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r897634018
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
If that is a concern, would it be a crazy idea to just check if the file exists?
That seems like a safer approach anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on PR #117:
URL: https://github.com/apache/flink-docker/pull/117#issuecomment-1156380284
> @nferrario actually, you should make this PR target for `dev-master` branch first, and we can cherry-pick the final commit to `dev-1.15` after code review if no obvious code conflict.
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on PR #117:
URL: https://github.com/apache/flink-docker/pull/117#issuecomment-1155221162
> These changes need to be applied to the dev-master/dev-1.15 branch. The master branch only contains generated content.
Will fix, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r897557823
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
I see.
Will `uname -i` always return `aarch64` on ARM platform?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r916412298
##########
docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
From the description of [uname](https://www.gnu.org/software/coreutils/manual/html_node/uname-invocation.html#uname-invocation), we can see `uname -m` is a better choice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r916711600
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
Thanks for the update. I said we can fall back to the default location, however, we still need to check whether the default location file existed. Otherwise, we need to print a warning message to tell users that even if we want to enable JEMALLOC, the dependency library does not exist.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r917814057
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
It makes me confused about why we need to disable `testing/testing_lib.sh` if in the current status.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on PR #117:
URL: https://github.com/apache/flink-docker/pull/117#issuecomment-1196358052
@nferrario would you please create PR targets for dev-1.14 and dev-1.15 branch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r896852520
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
I don't think we do. Not based on our tests at least. If we can generalize LD_PRELOAD, it should be platform agnostic (like my comment). Since RocksDB only supports x64 and ARM for now I don't think it's worth making it fully dynamic, also in case `uname -i` returns something other than `x86_64` on an actual x64 host.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r897886228
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
Since we're using a Debian-based image, `uname` should always return what we expect. The reason why I said `in case uname -i returns something other than x86_64 on an actual x64 host` is to NOT affect x64, not even by accident.
We can definitely check whether the file exists. I feel like `uname -i` is a cleaner approach, but I'm open to suggestions :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] KarlManong commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
KarlManong commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r912330737
##########
docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
why not using `$(uname -m)` directly in the line?
such as
```
export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -m)-linux-gnu/libjemalloc.so
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r917225904
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
> It already is a very low level thing that you won't be able to notice unless you specifically look for it, because there's no way to know that Jemalloc isn't being used (it took months for us to realize).
Actually, you can use `pmap <pid>` to check whether the process has attached the required library.
I think this PR looks good without unit test as the logic is quite easy, and it's a bit complex to verify this behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r927644004
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
I don't think it's a good idea to just disable the test directly, we can check the environment to decide whether to run test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka merged pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka merged PR #117:
URL: https://github.com/apache/flink-docker/pull/117
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r916657799
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
All good now! Thank you all for the patience and time to explain things. I certainly learned a few things and it'll help me for future contributions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r916652563
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
Yes I just made the changes and read this comment last, so I'm adding the fallback to that directory, although it looks like uname -m will never return something invalid
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r917771433
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
True, but for that we need to containerize the testing suite so that we can include those libs, and then have cross-platform testing with something like `docker buildx` (or mock those paths). I commented out the test and added a link to this thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] nferrario commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
nferrario commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r929213578
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
@Myasuka I just added that to the test. It's still useless until we move this to a Docker image, but we at least won't need to make a code change when we do so.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] Myasuka commented on a diff in pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #117:
URL: https://github.com/apache/flink-docker/pull/117#discussion_r896844883
##########
1.15/scala_2.12-java11-debian/docker-entrypoint.sh:
##########
@@ -91,7 +91,12 @@ prepare_configuration() {
maybe_enable_jemalloc() {
if [ "${DISABLE_JEMALLOC:-false}" == "false" ]; then
- export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
+ # Maybe use export LD_PRELOAD=$LD_PRELOAD:/usr/lib/$(uname -i)-linux-gnu/libjemalloc.so
+ if [[ `uname -i` == 'aarch64' ]]; then
+ export LD_PRELOAD=$LD_PRELOAD:/usr/lib/aarch64-linux-gnu/libjemalloc.so
Review Comment:
We need a `Dockerfile` could be built on arm platform successfully then the change in `docker-entrypoint.sh` is reasonable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-docker] zentol commented on pull request #117: [FLINK-28057] Fix LD_PRELOAD on ARM images
Posted by GitBox <gi...@apache.org>.
zentol commented on PR #117:
URL: https://github.com/apache/flink-docker/pull/117#issuecomment-1155219039
These changes need to be applied to the dev-master/dev-1.15 branch. The master branch only contains generated content.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org