Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Disable optimizations for knn library during docker build #384

Conversation

jmazanec15
Copy link
Member

@jmazanec15 jmazanec15 commented Sep 10, 2020

Issue #, if available:

  1. Fatal error during k-NN search k-NN#220
  2. Pass -march=x86-64 to build JNI library k-NN#164

Description of changes:
Remove architecture optimizations during docker build for knn library. Related PR in link above.

Test Results:
https://github.com/jmazanec15/opendistro-build/runs/1097770946?check_suite_focus=true#step:4:1471

Note: If this PR is related to Helm, please also update the README for related documentation changes. Thanks.
https://github.com/opendistro-for-elasticsearch/opendistro-build/blob/master/helm/README.md

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jmazanec15 jmazanec15 changed the title disable optimizations for knn Disable optimizations for knn Sep 10, 2020
@peterzhuamazon
Copy link
Contributor

@jmazanec15 could you please test this change with a workflow file. Thanks.

@jmazanec15
Copy link
Member Author

@peterzhuamazon Ran test docker build workflow here: 3604de6

Step 4 "Starting ES Docker Build" failed due to credential issue, because I ran the action from my fork, which does not have credentials setup, however, the k-NN library build passed:

2020-09-10T17:21:39.0560793Z [ 98%] Built target NonMetricSpaceLib
2020-09-10T17:21:39.0691753Z Scanning dependencies of target KNNIndexV2_0_6
2020-09-10T17:21:39.0815527Z [100%] Building CXX object CMakeFiles/KNNIndexV2_0_6.dir/src/com_amazon_opendistroforelasticsearch_knn_index_v206_KNNIndex.cpp.o
2020-09-10T17:21:39.4936608Z [91m/usr/share/elasticsearch/k-NN/jni/src/com_amazon_opendistroforelasticsearch_knn_index_v206_KNNIndex.cpp: In function 'jlong Java_com_amazon_opendistroforelasticsearch_knn_index_v206_KNNIndex_init(JNIEnv*, jclass, jstring, jobjectArray, jstring)':
2020-09-10T17:21:39.4937797Z /usr/share/elasticsearch/k-NN/jni/src/com_amazon_opendistroforelasticsearch_knn_index_v206_KNNIndex.cpp:215:12: warning: converting to non-pointer type 'long int' from NULL [-Wconversion-null]
2020-09-10T17:21:39.4938129Z      return NULL;
2020-09-10T17:21:39.4938347Z             ^
2020-09-10T17:21:40.5920005Z [0mLinking CXX shared library release/libKNNIndexV2_0_6.so
2020-09-10T17:21:40.9036542Z [100%] Built target KNNIndexV2_0_6
2020-09-10T17:21:40.9103765Z [91m+ mkdir /tmp/jni/
2020-09-10T17:21:40.9119002Z [0m[91m+ cp release/libKNNIndexV2_0_6.so /tmp/jni/
2020-09-10T17:21:40.9170933Z [0m[91m+ ls -ltr /tmp/jni/
2020-09-10T17:21:40.9184352Z [0mtotal 4780
2020-09-10T17:21:40.9184698Z -rwxr-xr-x 1 elasticsearch elasticsearch 4891168 Sep 10 17:21 libKNNIndexV2_0_6.so

Is this acceptable?

@jmazanec15 jmazanec15 changed the title Disable optimizations for knn Disable optimizations for knn library during docker build Sep 10, 2020
@peterzhuamazon
Copy link
Contributor

@peterzhuamazon Ran test docker build workflow here: 3604de6

Step 4 "Starting ES Docker Build" failed due to credential issue, because I ran the action from my fork, which does not have credentials setup, however, the k-NN library build passed:

2020-09-10T17:21:39.0560793Z [ 98%] Built target NonMetricSpaceLib
2020-09-10T17:21:39.0691753Z Scanning dependencies of target KNNIndexV2_0_6
2020-09-10T17:21:39.0815527Z [100%] Building CXX object CMakeFiles/KNNIndexV2_0_6.dir/src/com_amazon_opendistroforelasticsearch_knn_index_v206_KNNIndex.cpp.o
2020-09-10T17:21:39.4936608Z [91m/usr/share/elasticsearch/k-NN/jni/src/com_amazon_opendistroforelasticsearch_knn_index_v206_KNNIndex.cpp: In function 'jlong Java_com_amazon_opendistroforelasticsearch_knn_index_v206_KNNIndex_init(JNIEnv*, jclass, jstring, jobjectArray, jstring)':
2020-09-10T17:21:39.4937797Z /usr/share/elasticsearch/k-NN/jni/src/com_amazon_opendistroforelasticsearch_knn_index_v206_KNNIndex.cpp:215:12: warning: converting to non-pointer type 'long int' from NULL [-Wconversion-null]
2020-09-10T17:21:39.4938129Z      return NULL;
2020-09-10T17:21:39.4938347Z             ^
2020-09-10T17:21:40.5920005Z [0mLinking CXX shared library release/libKNNIndexV2_0_6.so
2020-09-10T17:21:40.9036542Z [100%] Built target KNNIndexV2_0_6
2020-09-10T17:21:40.9103765Z [91m+ mkdir /tmp/jni/
2020-09-10T17:21:40.9119002Z [0m[91m+ cp release/libKNNIndexV2_0_6.so /tmp/jni/
2020-09-10T17:21:40.9170933Z [0m[91m+ ls -ltr /tmp/jni/
2020-09-10T17:21:40.9184352Z [0mtotal 4780
2020-09-10T17:21:40.9184698Z -rwxr-xr-x 1 elasticsearch elasticsearch 4891168 Sep 10 17:21 libKNNIndexV2_0_6.so

Is this acceptable?

This is good. Thanks!

@peterzhuamazon peterzhuamazon merged commit d4c9cb3 into opendistro-for-elasticsearch:master Sep 10, 2020
@tanguilp
Copy link

Hi! Any trick to have the Docker image from the docker hub working before ODFE1.10.1 is released? Like setting CXXFLAGS env var somewhere...

Or is it required to build the image locally?

@gaiksaya
Copy link
Member

gaiksaya commented Sep 17, 2020

Hi @tanguilp ,
We will be soon releasing 1.10.1 as you have seen in our roadmap here. However, if you are not willing to wait, the work around this issue might be :

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants