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

Improve index correction performance #2234

Merged
merged 20 commits into from
Nov 22, 2023
Merged

Improve index correction performance #2234

merged 20 commits into from
Nov 22, 2023

Conversation

ykadowak
Copy link
Contributor

@ykadowak ykadowak commented Nov 10, 2023

Description:

  1. Add GetTimestamp rpc in agent. This is for reducing network bandwidth by not sending vector data itself, which is not required to correct index. This has a lot of impact when network quality is low.
  2. Changed to process agents by the reverse order of index numbers it has. The meaning of this is, by completing the process of a certain agent as soon as possible, it is possible to reduce the number of agents that need to be broadcasted at an early stage, thereby saving network bandwidth and reducing the waiting time for responses. This is effective when there is a bias in the number of indices each agent has.

Overall this PR reduces processing time by 30% compared to the worst case.

Related Issue:

Versions:

  • Go Version: 1.21.3
  • Docker Version: 20.10.8
  • Kubernetes Version: v1.28.2
  • NGT Version: 2.1.3

Checklist:

Special notes for your reviewer:

Sorry, something went wrong.

ykadowak and others added 5 commits November 10, 2023 02:42

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
This commit fixes the style issues introduced in 924903d according to the output
from Gofumpt and Prettier.

Details: #2234
Copy link

cloudflare-workers-and-pages bot commented Nov 10, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1625346
Status: ✅  Deploy successful!
Preview URL: https://7ee4ea0f.vald.pages.dev
Branch Preview URL: https://feature-corrector-faster2.vald.pages.dev

View logs

@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test


<a name="payload-v1-Info"></a>
Provides links to documentation or for performing an out of band action.
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
Did you mean “out-of-band”? (OUT_OF_PLACE[1])
Suggestions: out-of-band
URL: https://languagetool.org/insights/post/hyphen/#hyphenated-phrases-with-more-than-one-hyphen
Rule: https://community.languagetool.org/rule/show/OUT_OF_PLACE?lang=en-US&subId=1
Category: PUNCTUATION

Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 97 lines in your changes are missing coverage. Please review.

Comparison is base (3dc099a) 30.69% compared to head (1625346) 30.73%.

Files Patch % Lines
pkg/index/job/correction/service/corrector.go 9.23% 53 Missing and 6 partials ⚠️
internal/client/v1/client/agent/core/client.go 0.00% 25 Missing ⚠️
pkg/agent/core/ngt/handler/grpc/object.go 71.73% 10 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2234      +/-   ##
==========================================
+ Coverage   30.69%   30.73%   +0.04%     
==========================================
  Files         358      358              
  Lines       34996    35095      +99     
==========================================
+ Hits        10741    10786      +45     
- Misses      23752    23803      +51     
- Partials      503      506       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ykadowak ykadowak requested review from a team, hlts2 and datelier and removed request for a team November 15, 2023 08:48
@vdaas-ci
Copy link
Collaborator

Profile Report

typevald-agent-ngtvald-lb-gatewayvald-discoverervald-manager-index
cpu
heap
other images

@ykadowak ykadowak requested a review from hlts2 November 21, 2023 06:57
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ykadowak ykadowak merged commit dcf2064 into main Nov 22, 2023
88 of 89 checks passed
@ykadowak ykadowak deleted the feature/corrector/faster2 branch November 22, 2023 00:40
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
@vankichi vankichi mentioned this pull request Dec 4, 2023
kmrmt pushed a commit that referenced this pull request Dec 12, 2023
* Add GetObjectMeta rpc in proto

* Compile GetObjectMeta

* Implement GetObjectMeta

* Add test for GetObjectMeta

* style: format code with Gofumpt and Prettier

This commit fixes the style issues introduced in 924903d according to the output
from Gofumpt and Prettier.

Details: #2234

* Add GetObjectMeta client

* Change to use GetObjectMeta in corrector logic

* Rename rpc to GetTimestamp

* Update test to check all the data set

* Remove unused code in correct function

* Add error handling for empty returned vectors in
stream test

* Add fillVectorField when there is none

* Reverse agentAddrs to decrease broadcast

* process the reverse order of number of indexes

* Refactor loadInfos function to loadAgentIndexInfo

* declare slice as empty slice for deepsource

* Add comment

* Add eg.Wait

---------

Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
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.

None yet

4 participants