Skip to content

Commit

Permalink
Update package name to fix compilation issue (#2513)
Browse files Browse the repository at this point in the history
* Update package name to fix compilation issue

Core renamed this package in opensearch-project/OpenSearch#17272
This commit renames package accordingly

Signed-off-by: Balasubramanian <balasvij@amazon.com>

* Update build.gradle and build.sh to separate x64 linux nmslib build with different gcc versions (#2506) (#2508)

* Update build gradle to separate nmslib / faiss generation

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

* Update scripts/build.sh to separate enable gcc switch

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

* Remove test comments

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

* Remove test comments

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

* Remove test comments

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

* Updating restart and rolling upgrade bwc test bundle.gradle

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

* Enforce gcc10 for nmslib to compile and avx512_spr have no-op

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

---------

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
(cherry picked from commit 107c4f1)

Co-authored-by: Peter Zhu <zhujiaxi@amazon.com>

---------

Signed-off-by: Balasubramanian <balasvij@amazon.com>
Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com>
Co-authored-by: Peter Zhu <zhujiaxi@amazon.com>
  • Loading branch information
3 people authored Feb 10, 2025
1 parent d83057d commit b770281
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ jobs:
matrix:
java: [21, 23]

env:
CC: gcc10-gcc
CXX: gcc10-g++
FC: gcc10-gfortran

name: Build and Test k-NN Plugin on Linux
runs-on: ubuntu-latest
needs: Get-CI-Image-Tag
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/test_security.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ jobs:
strategy:
matrix:
java: [21]
env:
CC: gcc10-gcc
CXX: gcc10-g++
FC: gcc10-gfortran

name: Run Integration Tests on Linux
runs-on: ubuntu-latest
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
* Upgrade min JDK compatibility to JDK 21 [#2422](https://github.com/opensearch-project/k-NN/pull/2422)
### Documentation
### Maintenance
* Update package name to fix compilation issue [#2513](https://github.com/opensearch-project/k-NN/pull/2513)
### Refactoring

## [Unreleased 2.x](https://github.com/opensearch-project/k-NN/compare/2.18...2.x)
Expand Down
20 changes: 18 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -405,22 +405,36 @@ tasks.register('cmakeJniLib', Exec) {
standardOutput = outputStream
}

task buildNmslib(type:Exec) {
dependsOn cmakeJniLib
def makePath = findExecutable("make")
logger.lifecycle("Using make at: ${makePath}")
// Ensure makePath is treated as a String. If parsing to an int is required,
// handle it explicitly, though a path typically should not be parsed as an int.
if (makePath.isEmpty()) {
throw new GradleException("Make not found in PATH. Please install Make.")
}
def outputStream = new ByteArrayOutputStream()
commandLine makePath, '-Cjni/build', 'opensearchknn_nmslib', '-j', "${nproc_count}"
standardOutput = outputStream
}

task buildJniLib(type:Exec) {
dependsOn cmakeJniLib
def makePath = findExecutable("make")
logger.lifecycle("Using make at: ${makePath}")
println "Make path: ${makePath}"
// Ensure makePath is treated as a String. If parsing to an int is required,
// handle it explicitly, though a path typically should not be parsed as an int.
if (makePath.isEmpty()) {
throw new GradleException("Make not found in PATH. Please install Make.")
}
def outputStream = new ByteArrayOutputStream()
commandLine makePath, '-Cjni/build','opensearchknn_nmslib', 'opensearchknn_faiss', 'opensearchknn_common', '-j', "${nproc_count}"
commandLine makePath, '-Cjni/build', 'opensearchknn_faiss', 'opensearchknn_common', '-j', "${nproc_count}"
standardOutput = outputStream
}

test {
dependsOn buildNmslib
dependsOn buildJniLib
systemProperty 'tests.security.manager', 'false'
systemProperty "java.library.path", "$rootDir/jni/release"
Expand All @@ -435,6 +449,7 @@ test {
def _numNodes = findProperty('numNodes') as Integer ?: 1
integTest {
if (integTestDependOnJniLib) {
dependsOn buildNmslib
dependsOn buildJniLib
}
systemProperty 'tests.security.manager', 'false'
Expand Down Expand Up @@ -533,6 +548,7 @@ task integTestRemote(type: RestIntegTestTask) {

run {
useCluster project.testClusters.integTest
dependsOn buildNmslib
dependsOn buildJniLib
doFirst {
// There seems to be an issue when running multi node run or integ tasks with unicast_hosts
Expand Down
1 change: 1 addition & 0 deletions qa/restart-upgrade/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ testClusters {
// All nodes are upgraded to latest version and run the tests
task testRestartUpgrade(type: StandaloneRestIntegTestTask) {
dependsOn "testAgainstOldCluster"
dependsOn rootProject.tasks.buildNmslib
dependsOn rootProject.tasks.buildJniLib
dependsOn rootProject.tasks.assemble
useCluster testClusters."${baseName}"
Expand Down
1 change: 1 addition & 0 deletions qa/rolling-upgrade/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ task testAgainstOldCluster(type: StandaloneRestIntegTestTask) {
// This results in a mixed cluster with 2 nodes on the old version and 1 upgraded node.
task testAgainstOneThirdUpgradedCluster(type: StandaloneRestIntegTestTask) {
useCluster testClusters."${baseName}"
dependsOn rootProject.tasks.buildNmslib
dependsOn rootProject.tasks.buildJniLib
dependsOn rootProject.tasks.assemble
dependsOn "testAgainstOldCluster"
Expand Down
10 changes: 9 additions & 1 deletion scripts/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -133,16 +133,24 @@ cd $work_dir
./gradlew :buildJniLib -Davx512.enabled=false -Davx512_spr.enabled=false -Davx2.enabled=false -Dbuild.lib.commit_patches=false -Dnproc.count=${NPROC_COUNT:-1}

if [ "$PLATFORM" != "windows" ] && [ "$ARCHITECTURE" = "x64" ]; then
echo "Building k-NN library nmslib with gcc 10 on non-windows x64"
rm -rf jni/CMakeCache.txt jni/CMakeFiles
env CC=gcc10-gcc CXX=gcc10-g++ FC=gcc10-gfortran ./gradlew :buildNmslib -Dbuild.lib.commit_patches=false -Dbuild.lib.apply_patches=false

echo "Building k-NN library after enabling AVX2"
# Skip applying patches as patches were applied already from previous :buildJniLib task
# If we apply patches again, it fails with conflict
rm -rf jni/CMakeCache.txt jni/CMakeFiles
./gradlew :buildJniLib -Davx2.enabled=true -Davx512.enabled=false -Davx512_spr.enabled=false -Dbuild.lib.commit_patches=false -Dbuild.lib.apply_patches=false

echo "Building k-NN library after enabling AVX512"
./gradlew :buildJniLib -Davx512.enabled=true -Davx512_spr.enabled=false -Dbuild.lib.commit_patches=false -Dbuild.lib.apply_patches=false

echo "Building k-NN library after enabling AVX512_SPR"
./gradlew :buildJniLib -Davx512_spr.enabled=true -Dbuild.lib.commit_patches=false -Dbuild.lib.apply_patches=false

else
./gradlew :buildNmslib -Dbuild.lib.commit_patches=false -Dbuild.lib.apply_patches=false
fi

./gradlew publishPluginZipPublicationToZipStagingRepository -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER
Expand All @@ -151,7 +159,7 @@ fi
# Add lib to zip
zipPath=$(find "$(pwd)/build/distributions" -path \*.zip)
distributions="$(dirname "${zipPath}")"
mkdir $distributions/lib
mkdir -p $distributions/lib
libPrefix="libopensearchknn"
if [ "$PLATFORM" = "windows" ]; then
libPrefix="opensearchknn"
Expand Down

0 comments on commit b770281

Please sign in to comment.