Skip to content
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

Improvement in release perf test #1243

Merged
merged 1 commit into from
Oct 12, 2023
Merged

Conversation

heemin32
Copy link
Collaborator

@heemin32 heemin32 commented Oct 12, 2023

Description

Simply the process to run performance test for release

Previous behavior

  1. Update cluster setting with "knn.algo_param.index_thread_qty" : 4
  2. Download hdf5 data
  3. Modify test template with correct value of endpoint, port, path to json files, and path to hdf5 files manually.
  4. Run knn-per-tool.py
  5. Repeat step2 and step3 for 12 test cases

After change

  1. Download hdf5 data in k-NN/benchmakrs/perf-tool/dataset/
  2. Run run_all_tests.sh --endpoint <your_endpoint>

Issues Resolved

N/A

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed as per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@heemin32 heemin32 changed the title Improvement in release test Improvement in release perf test Oct 12, 2023
Comment on lines +69 to +80
TESTS="./release-configs/faiss-hnsw/filtering/relaxed-filter/relaxed-filter-test.yml
./release-configs/faiss-hnsw/filtering/restrictive-filter/restrictive-filter-test.yml
./release-configs/faiss-hnsw/test.yml
./release-configs/faiss-hnswpq/test.yml
./release-configs/faiss-ivf/filtering/relaxed-filter/relaxed-filter-test.yml
./release-configs/faiss-ivf/filtering/restrictive-filter/restrictive-filter-test.yml
./release-configs/faiss-ivf/test.yml
./release-configs/faiss-ivfpq/test.yml
./release-configs/lucene-hnsw/filtering/relaxed-filter/relaxed-filter-test.yml
./release-configs/lucene-hnsw/filtering/restrictive-filter/restrictive-filter-test.yml
./release-configs/lucene-hnsw/test.yml
./release-configs/nmslib-hnsw/test.yml"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add some sleep time between running of all the tests to make CPU come down and cluster to become stable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add some sleep time between running of all the tests to make CPU come down and cluster to become stable.

Do you think 1 min will be enough for that purpose?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah 1 min is fine.

@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Merging #1243 (1fa7a8f) into main (34d697f) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #1243   +/-   ##
=========================================
  Coverage     85.07%   85.07%           
  Complexity     1210     1210           
=========================================
  Files           160      160           
  Lines          4932     4932           
  Branches        449      449           
=========================================
  Hits           4196     4196           
  Misses          537      537           
  Partials        199      199           

@navneet1v
Copy link
Collaborator

@heemin32 do you know why this [k-NN Rolling-Upgrade BWC Tests (17, ubuntu-latest, 2.10.0-SNAPSHOT, 3.0.0-SNAPSHOT)](https://github.com/opensearch-project/k-NN/actions/runs/6489712314/job/17624422073?pr=1243#logs) is coming even when we have removed to 2.10.0 from 2.10.0-SNAPSHOT? here: 34d697f

@heemin32
Copy link
Collaborator Author

@heemin32 do you know why this [k-NN Rolling-Upgrade BWC Tests (17, ubuntu-latest, 2.10.0-SNAPSHOT, 3.0.0-SNAPSHOT)](https://github.com/opensearch-project/k-NN/actions/runs/6489712314/job/17624422073?pr=1243#logs) is coming even when we have removed to 2.10.0 from 2.10.0-SNAPSHOT? here: 34d697f

#1245

@navneet1v
Copy link
Collaborator

@heemin32 do you know why this [k-NN Rolling-Upgrade BWC Tests (17, ubuntu-latest, 2.10.0-SNAPSHOT, 3.0.0-SNAPSHOT)](https://github.com/opensearch-project/k-NN/actions/runs/6489712314/job/17624422073?pr=1243#logs) is coming even when we have removed to 2.10.0 from 2.10.0-SNAPSHOT? here: 34d697f

#1245

thanks for raising the PR

navneet1v
navneet1v previously approved these changes Oct 12, 2023
@martin-gaievski
Copy link
Member

curious if with every next run time changes, as this all is on same cluster, I think some query stuff may be cached after multiple runs?
I think we may extend it in future by allowing different endpoints/clusters for different tests.

@navneet1v
Copy link
Collaborator

curious if with every next run time changes, as this all is on same cluster, I think some query stuff may be cached after multiple runs?
I think we may extend it in future by allowing different endpoints/clusters for different tests.

@martin-gaievski queries won’t be cached because we delete the index at the start of every run.
the only thing that may cause issue is RSS. But generally we run with 4xlarges so that shouldn’t not be a problem too

@heemin32
Copy link
Collaborator Author

curious if with every next run time changes, as this all is on same cluster, I think some query stuff may be cached after multiple runs? I think we may extend it in future by allowing different endpoints/clusters for different tests.

This is what we have been doing for release benchmark test as long as I know. We are not using different cluster for each test run.

@heemin32 heemin32 merged commit 5692c2a into opensearch-project:main Oct 12, 2023
38 of 44 checks passed
@heemin32 heemin32 deleted the test branch October 12, 2023 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants