-
Notifications
You must be signed in to change notification settings - Fork 3k
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
enhance: [2.4] add metrics for counting number of nun-zeros/tokens of sparse search #38328
enhance: [2.4] add metrics for counting number of nun-zeros/tokens of sparse search #38328
Conversation
Signed-off-by: Buqian Zheng <[email protected]>
@zhengbuqian Please associate the related pr of master to the body of your Pull Request. (eg. “pr: #”) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 2.4 #38328 +/- ##
==========================================
- Coverage 80.41% 72.14% -8.27%
==========================================
Files 1074 1074
Lines 168774 168776 +2
==========================================
- Hits 135713 121768 -13945
- Misses 28613 42565 +13952
+ Partials 4448 4443 -5
|
@@ -83,14 +82,10 @@ func fieldDataToPlaceholderValue(fieldData *schemapb.FieldData) (*commonpb.Place | |||
return nil, errors.New("vector data is not schemapb.VectorField_SparseFloatVector") | |||
} | |||
vec := vectors.SparseFloatVector | |||
bytes, err := proto.Marshal(vec) |
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.
What is the reason for removing this Marshal
?
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.
it was a mistake: PlaceholderValue::Values should be just the byte representation of sparse vector, not some serialized proto. no one has ever used this search by pk feature thus this bug is never triggered.
/lgtm |
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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: congqixia, zhengbuqian The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…e/FTS search (#38329) sparse vectors may have arbitrary number of non zeros and it is hard to optimize without knowing the actual distribution of nnz. this PR adds a metric for analyzing that. issue: #35853 comparing with #38328, this includes also metric for FTS in query node delegator also fixed a bug of sparse when searching by pk Signed-off-by: Buqian Zheng <[email protected]>
sparse vectors may have arbitrary number of non zeros and it is hard to optimize without knowing the actual distribution of nnz. this PR adds a metric for analyzing that.
pr: #38329
also fixed a bug of sparse when searching by pk