-
Notifications
You must be signed in to change notification settings - Fork 476
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
Fix exponential query sharding time of binops #3027
Fix exponential query sharding time of binops #3027
Conversation
When sharding operations like `expr1 or expr2 or ... or expr_n`, the last expression has to be cloned 2^n times by the `canShardAllVectorSelectors` check. We can avoid that by caching the results of that function for each expr, making the execution time linear again. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM
* Fix exponential query sharding time of binops When sharding operations like `expr1 or expr2 or ... or expr_n`, the last expression has to be cloned 2^n times by the `canShardAllVectorSelectors` check. We can avoid that by caching the results of that function for each expr, making the execution time linear again. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update CHANGELOG.md Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update test to check 30 expressions Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
* Fix exponential query sharding time of binops When sharding operations like `expr1 or expr2 or ... or expr_n`, the last expression has to be cloned 2^n times by the `canShardAllVectorSelectors` check. We can avoid that by caching the results of that function for each expr, making the execution time linear again. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update CHANGELOG.md Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update test to check 30 expressions Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
if can, ok := summer.canShardAllVectorSelectorsCache[query]; ok { | ||
return can | ||
} | ||
defer func() { summer.canShardAllVectorSelectorsCache[query] = can }() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is this caching the result of the function which is c.stats.GetShardedQueries() == countVectorSelectors(mappedExpr)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can
is a named return value. So return
statement assigns the value to it, and then the defer
statement is executed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ugh, magic, thanks
* Fix exponential query sharding time of binops When sharding operations like `expr1 or expr2 or ... or expr_n`, the last expression has to be cloned 2^n times by the `canShardAllVectorSelectors` check. We can avoid that by caching the results of that function for each expr, making the execution time linear again. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update CHANGELOG.md Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update test to check 30 expressions Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
* Fix exponential query sharding time of binops When sharding operations like `expr1 or expr2 or ... or expr_n`, the last expression has to be cloned 2^n times by the `canShardAllVectorSelectors` check. We can avoid that by caching the results of that function for each expr, making the execution time linear again. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update CHANGELOG.md Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update test to check 30 expressions Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-3027-to-release-2.3 origin/release-2.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 0e45c39fa33ddefe0ceea402a6d15a21b46f4752
# Push it to GitHub
git push --set-upstream origin backport-3027-to-release-2.3
git switch main
# Remove the local backport branch
git branch -D backport-3027-to-release-2.3 Then, create a pull request where the |
* Fix exponential query sharding time of binops When sharding operations like `expr1 or expr2 or ... or expr_n`, the last expression has to be cloned 2^n times by the `canShardAllVectorSelectors` check. We can avoid that by caching the results of that function for each expr, making the execution time linear again. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update CHANGELOG.md Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update test to check 30 expressions Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> (cherry picked from commit 0e45c39)
* Fix exponential query sharding time of binops When sharding operations like `expr1 or expr2 or ... or expr_n`, the last expression has to be cloned 2^n times by the `canShardAllVectorSelectors` check. We can avoid that by caching the results of that function for each expr, making the execution time linear again. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update CHANGELOG.md Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update test to check 30 expressions Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> (cherry picked from commit 0e45c39) Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com>
What this PR does
When sharding operations like
expr1 or expr2 or ... or expr_n
, the last expression has to be cloned2^n
times by thecanShardAllVectorSelectors
check.We can avoid that by caching the results of that function for each expr, making the execution time linear again.
Which issue(s) this PR fixes or relates to
Fixes an internal issue.
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]