diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 8795ac319..884a36be0 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -38,12 +38,6 @@ jobs: with: submodules: true - # Setup git user so that patches for native libraries can be applied and committed - - name: Setup git user - run: | - su `id -un 1000` -c 'git config --global user.name "github-actions[bot]"' - su `id -un 1000` -c 'git config --global user.email "github-actions[bot]@users.noreply.github.com"' - - name: Setup Java ${{ matrix.java }} uses: actions/setup-java@v1 with: @@ -56,10 +50,10 @@ jobs: if lscpu | grep -i avx2 then echo "avx2 available on system" - su `id -un 1000` -c "whoami && java -version && ./gradlew build" + su `id -un 1000` -c "whoami && java -version && ./gradlew build -Dbuild.lib.commit_patches=false" else echo "avx2 not available on system" - su `id -un 1000` -c "whoami && java -version && ./gradlew build -Dsimd.enabled=false" + su `id -un 1000` -c "whoami && java -version && ./gradlew build -Dsimd.enabled=false -Dbuild.lib.commit_patches=false" fi @@ -81,12 +75,6 @@ jobs: - name: Checkout k-NN uses: actions/checkout@v1 - # Setup git user so that patches for native libraries can be applied and committed - - name: Setup git user - run: | - git config --global user.name "github-actions[bot]" - git config --global user.email "github-actions[bot]@users.noreply.github.com" - - name: Setup Java ${{ matrix.java }} uses: actions/setup-java@v1 with: @@ -102,10 +90,10 @@ jobs: if sysctl -n machdep.cpu.features machdep.cpu.leaf7_features | grep -i AVX2 then echo "avx2 available on system" - ./gradlew build + ./gradlew build -Dbuild.lib.commit_patches=false else echo "avx2 not available on system" - ./gradlew build -Dsimd.enabled=false + ./gradlew build -Dsimd.enabled=false -Dbuild.lib.commit_patches=false fi Build-k-NN-Windows: @@ -121,12 +109,6 @@ jobs: - name: Checkout k-NN uses: actions/checkout@v1 - # Setup git user so that patches for native libraries can be applied and committed - - name: Setup git user - run: | - git config --global user.name "github-actions[bot]" - git config --global user.email "github-actions[bot]@users.noreply.github.com" - - name: Setup Java ${{ matrix.java }} uses: actions/setup-java@v1 with: @@ -165,5 +147,5 @@ jobs: - name: Run build run: | - ./gradlew.bat build -D'simd.enabled=false' + ./gradlew.bat build -D'simd.enabled=false' -D'build.lib.commit_patches=false' diff --git a/.github/workflows/backwards_compatibility_tests_workflow.yml b/.github/workflows/backwards_compatibility_tests_workflow.yml index cfa1bb128..d46da9e2b 100644 --- a/.github/workflows/backwards_compatibility_tests_workflow.yml +++ b/.github/workflows/backwards_compatibility_tests_workflow.yml @@ -36,12 +36,6 @@ jobs: - name: Checkout k-NN uses: actions/checkout@v1 - # Setup git user so that patches for native libraries can be applied and committed - - name: Setup git user - run: | - git config --global user.name "github-actions[bot]" - git config --global user.email "github-actions[bot]@users.noreply.github.com" - - name: Setup Java ${{ matrix.java }} uses: actions/setup-java@v1 with: @@ -80,13 +74,13 @@ jobs: name: Run k-NN Restart-Upgrade BWC Tests from BWCVersion-${{ matrix.bwc_version }} to OpenSearch Version-${{ matrix.opensearch_version }} on Windows run: | echo "Running restart-upgrade backwards compatibility tests ..." - ./gradlew :qa:restart-upgrade:testRestartUpgrade -D'tests.bwc.version=${{ matrix.bwc_version }}' + ./gradlew :qa:restart-upgrade:testRestartUpgrade -D'tests.bwc.version=${{ matrix.bwc_version }}' -D'build.lib.commit_patches=false' - if: startsWith(matrix.os,'ubuntu') name: Run k-NN Restart-Upgrade BWC Tests from BWCVersion-${{ matrix.bwc_version }} to OpenSearch Version-${{ matrix.opensearch_version }} on Ubuntu run: | echo "Running restart-upgrade backwards compatibility tests ..." - ./gradlew :qa:restart-upgrade:testRestartUpgrade -Dtests.bwc.version=$BWC_VERSION_RESTART_UPGRADE + ./gradlew :qa:restart-upgrade:testRestartUpgrade -Dtests.bwc.version=$BWC_VERSION_RESTART_UPGRADE -Dbuild.lib.commit_patches=false Rolling-Upgrade-BWCTests-k-NN: @@ -106,12 +100,6 @@ jobs: - name: Checkout k-NN uses: actions/checkout@v1 - # Setup git user so that patches for native libraries can be applied and committed - - name: Setup git user - run: | - git config --global user.name "github-actions[bot]" - git config --global user.email "github-actions[bot]@users.noreply.github.com" - - name: Setup Java ${{ matrix.java }} uses: actions/setup-java@v1 with: @@ -150,10 +138,10 @@ jobs: name: Run k-NN Rolling-Upgrade BWC Tests from BWCVersion-${{ matrix.bwc_version }} to OpenSearch Version-${{ matrix.opensearch_version }} on Windows run: | echo "Running rolling-upgrade backwards compatibility tests ..." - ./gradlew :qa:rolling-upgrade:testRollingUpgrade -D'tests.bwc.version=${{ matrix.bwc_version }}' + ./gradlew :qa:rolling-upgrade:testRollingUpgrade -D'tests.bwc.version=${{ matrix.bwc_version }}' -D'build.lib.commit_patches=false' - if: startsWith(matrix.os,'ubuntu') name: Run k-NN Rolling-Upgrade BWC Tests from BWCVersion-${{ matrix.bwc_version }} to OpenSearch Version-${{ matrix.opensearch_version }} on Ubuntu run: | echo "Running rolling-upgrade backwards compatibility tests ..." - ./gradlew :qa:rolling-upgrade:testRollingUpgrade -Dtests.bwc.version=$BWC_VERSION_ROLLING_UPGRADE + ./gradlew :qa:rolling-upgrade:testRollingUpgrade -Dtests.bwc.version=$BWC_VERSION_ROLLING_UPGRADE -Dbuild.lib.commit_patches=false diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 78206bada..31a1cd8aa 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -37,11 +37,6 @@ jobs: uses: actions/checkout@v1 with: submodules: true - # Setup git user so that patches for native libraries can be applied and committed - - name: Setup git user - run: | - su `id -un 1000` -c 'git config --global user.name "github-actions[bot]"' - su `id -un 1000` -c 'git config --global user.email "github-actions[bot]@users.noreply.github.com"' - name: Setup Java ${{ matrix.java }} uses: actions/setup-java@v1 @@ -52,4 +47,4 @@ jobs: # switching the user, as OpenSearch cluster can only be started as root/Administrator on linux-deb/linux-rpm/windows-zip. run: | chown -R 1000:1000 `pwd` - su `id -un 1000` -c "whoami && java -version && ./gradlew integTest -Dsecurity.enabled=true -Dsimd.enabled=true" + su `id -un 1000` -c "whoami && java -version && ./gradlew integTest -Dsecurity.enabled=true -Dsimd.enabled=true -Dbuild.lib.commit_patches=false" diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index ca5e7cc52..9c17f4f68 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -237,6 +237,13 @@ If you want to make a custom patch on JNI library 3. Place the patch file under `jni/patches` 4. Make a change in `jni/CmakeLists.txt`, `.github/workflows/CI.yml` to apply the patch during build +By default, in the cmake build system, these patches will be applied and committed to the native libraries. In order to +successfully make the commits the `user.name` and `user.email` git configurations need to be setup. If you cannot set +these in your environment, you can disable committing the changes to the library by passing gradle this flag: +`build.lib.commit_patches=false`. For example, `gradlew build -Dbuild.lib.commit_patches=false`. If the patches are +not committed, then the full library build process will run each time `cmake` is invoked. In a development environment, +it is recommended to setup the user git configuration to avoid this cost. + ### Enable SIMD Optimization SIMD(Single Instruction/Multiple Data) Optimization is enabled by default on Linux and Mac which boosts the performance by enabling `AVX2` on `x86 architecture` and `NEON` on `ARM64 architecture` while building the Faiss library. But to enable SIMD, the underlying processor diff --git a/build.gradle b/build.gradle index cb29b93b8..bfdc30967 100644 --- a/build.gradle +++ b/build.gradle @@ -18,6 +18,9 @@ buildscript { opensearch_group = "org.opensearch" isSnapshot = "true" == System.getProperty("build.snapshot", "true") simd_enabled = System.getProperty("simd.enabled", "true") + // Flag to determine whether cmake build system should apply patches and commit. In automated build environments + // set this to false. In dev environments, set to true + commit_lib_patches = System.getProperty("build.lib.commit_patches", "true") version_tokens = opensearch_version.tokenize('-') opensearch_build = version_tokens[0] + '.0' @@ -303,10 +306,10 @@ task cmakeJniLib(type:Exec) { workingDir 'jni' if (Os.isFamily(Os.FAMILY_WINDOWS)) { dependsOn windowsPatches - commandLine 'cmake', '.', "-G", "Unix Makefiles", "-DKNN_PLUGIN_VERSION=${opensearch_version}", "-DBLAS_LIBRARIES=$rootDir\\src\\main\\resources\\windowsDependencies\\libopenblas.dll", "-DLAPACK_LIBRARIES=$rootDir\\src\\main\\resources\\windowsDependencies\\libopenblas.dll", "-DSIMD_ENABLED=${simd_enabled}" + commandLine 'cmake', '.', "-G", "Unix Makefiles", "-DKNN_PLUGIN_VERSION=${opensearch_version}", "-DBLAS_LIBRARIES=$rootDir\\src\\main\\resources\\windowsDependencies\\libopenblas.dll", "-DLAPACK_LIBRARIES=$rootDir\\src\\main\\resources\\windowsDependencies\\libopenblas.dll", "-DSIMD_ENABLED=${simd_enabled}", "-DCOMMIT_LIB_PATCHES=${commit_lib_patches}" } else { - commandLine 'cmake', '.', "-DKNN_PLUGIN_VERSION=${opensearch_version}", "-DSIMD_ENABLED=${simd_enabled}" + commandLine 'cmake', '.', "-DKNN_PLUGIN_VERSION=${opensearch_version}", "-DSIMD_ENABLED=${simd_enabled}", "-DCOMMIT_LIB_PATCHES=${commit_lib_patches}" } } diff --git a/jni/CMakeLists.txt b/jni/CMakeLists.txt index 3429671fe..6f0894607 100644 --- a/jni/CMakeLists.txt +++ b/jni/CMakeLists.txt @@ -31,6 +31,18 @@ else() set(CONFIG_ALL OFF) endif () +# `git am` will create commits from the patches in the native libraries. This is ideal for development envs +# because it prevents full lib rebuild everytime cmake is run. However, for build systems that will run the +# build workflow once, it can cause issues because git commits require that the user and the user's email be set. +# See https://github.com/opensearch-project/k-NN/issues/1651. So, we provide a flag that allows users to select between +# the two +if(NOT DEFINED COMMIT_LIB_PATCHES OR ${COMMIT_LIB_PATCHES} STREQUAL true) + set(GIT_PATCH_COMMAND am) +else() + set(GIT_PATCH_COMMAND apply) +endif() +message(STATUS "Using the following git patch command: \"${GIT_PATCH_COMMAND}\"") + # Set OS specific variables if (${CMAKE_SYSTEM_NAME} STREQUAL Darwin) set(CMAKE_MACOSX_RPATH 1) diff --git a/jni/cmake/init-faiss.cmake b/jni/cmake/init-faiss.cmake index 44dd4442a..fefed61e2 100644 --- a/jni/cmake/init-faiss.cmake +++ b/jni/cmake/init-faiss.cmake @@ -18,8 +18,8 @@ find_path(PATCH_FILE NAMES 0001-Custom-patch-to-support-multi-vector.patch 0002- # If it exists, apply patches if (EXISTS ${PATCH_FILE}) message(STATUS "Applying custom patches.") - execute_process(COMMAND git am --3way ${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0001-Custom-patch-to-support-multi-vector.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE) - execute_process(COMMAND git am --3way ${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0002-Enable-precomp-table-to-be-shared-ivfpq.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE) + execute_process(COMMAND git ${GIT_PATCH_COMMAND} --3way --ignore-space-change --ignore-whitespace ${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0001-Custom-patch-to-support-multi-vector.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE) + execute_process(COMMAND git ${GIT_PATCH_COMMAND} --3way --ignore-space-change --ignore-whitespace ${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0002-Enable-precomp-table-to-be-shared-ivfpq.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE) if(RESULT_CODE) message(FATAL_ERROR "Failed to apply patch:\n${ERROR_MSG}") endif() diff --git a/jni/cmake/init-nmslib.cmake b/jni/cmake/init-nmslib.cmake index edf2296d9..f6fe641d1 100644 --- a/jni/cmake/init-nmslib.cmake +++ b/jni/cmake/init-nmslib.cmake @@ -15,10 +15,14 @@ endif () # Check if patch exist, this is to skip git apply during CI build. See CI.yml with ubuntu. find_path(PATCH_FILE NAMES 0001-Initialize-maxlevel-during-add-from-enterpoint-level.patch PATHS ${CMAKE_CURRENT_SOURCE_DIR}/patches/nmslib NO_DEFAULT_PATH) +#TODO: Debugging TO BE REMOVED +execute_process(COMMAND pwd WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib) +execute_process(COMMAND ls -la WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib) + # If it exists, apply patches if (EXISTS ${PATCH_FILE}) message(STATUS "Applying custom patches.") - execute_process(COMMAND git am --3way ${CMAKE_CURRENT_SOURCE_DIR}/patches/nmslib/0001-Initialize-maxlevel-during-add-from-enterpoint-level.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE) + execute_process(COMMAND git ${GIT_PATCH_COMMAND} --3way --ignore-space-change --ignore-whitespace ${CMAKE_CURRENT_SOURCE_DIR}/patches/nmslib/0001-Initialize-maxlevel-during-add-from-enterpoint-level.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE) if(RESULT_CODE) message(FATAL_ERROR "Failed to apply patch:\n${ERROR_MSG}") diff --git a/scripts/build.sh b/scripts/build.sh index 43a36443d..d2571efb6 100644 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -118,12 +118,12 @@ fi # Build k-NN lib and plugin through gradle tasks cd $work_dir -./gradlew build --no-daemon --refresh-dependencies -x integTest -x test -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER -./gradlew :buildJniLib -Dsimd.enabled=false +./gradlew build --no-daemon --refresh-dependencies -x integTest -x test -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER -Dbuild.lib.commit_patches=false +./gradlew :buildJniLib -Dsimd.enabled=false -Dbuild.lib.commit_patches=false if [ "$PLATFORM" != "windows" ] && [ "$ARCHITECTURE" = "x64" ]; then echo "Building k-NN library after enabling AVX2" - ./gradlew :buildJniLib -Dsimd.enabled=true + ./gradlew :buildJniLib -Dsimd.enabled=true -Dbuild.lib.commit_patches=false fi ./gradlew publishPluginZipPublicationToZipStagingRepository -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER