feat: supporing hybrid search group_by #35982

MrPresent-Han · 2024-09-04T10:53:27Z

related: #35096

mergify · 2024-09-04T10:54:04Z

Invalid PR Title Format Detected

Your PR submission does not adhere to our required standards. To ensure clarity and consistency, please meet the following criteria:

Title Format: The PR title must begin with one of these prefixes:

feat: for introducing a new feature.
fix: for bug fixes.
enhance: for improvements to existing functionality.
test: for add tests to existing functionality.
doc: for modifying documentation.
auto: for the pull request from bot.

Description Requirement: The PR must include a non-empty description, detailing the changes and their impact.

Required Title Structure:

[Type]: [Description of the PR]

Where Type is one of feat, fix, enhance, test or doc.

Example:

enhance: improve search performance significantly

Please review and update your PR to comply with these guidelines.

mergify · 2024-09-04T11:02:00Z

@MrPresent-Han E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-09-04T12:38:26Z

@MrPresent-Han E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

Signed-off-by: MrPresent-Han <chun.han@gmail.com>

mergify · 2024-09-04T14:14:51Z

@MrPresent-Han E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

Signed-off-by: MrPresent-Han <chun.han@gmail.com>

codecov · 2024-09-05T06:20:09Z

Codecov Report

Attention: Patch coverage is 85.71429% with 55 lines in your changes missing coverage. Please review.

Project coverage is 72.63%. Comparing base (5247631) to head (4b86fe6).
Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/proxy/search_reduce_util.go	82.94%	25 Missing and 19 partials ⚠️
internal/querynodev2/services.go	83.33%	2 Missing and 1 partial ⚠️
internal/proxy/task_search.go	91.66%	1 Missing and 1 partial ⚠️
internal/querynodev2/segments/result.go	80.00%	1 Missing and 1 partial ⚠️
internal/querynodev2/segments/search_reduce.go	90.00%	0 Missing and 2 partials ⚠️
internal/util/reduce/reduce_info.go	95.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #35982      +/-   ##
==========================================
- Coverage   81.57%   72.63%   -8.94%     
==========================================
  Files        1266     1267       +1     
  Lines      150740   150904     +164     
==========================================
- Hits       122959   109609   -13350     
- Misses      22911    36409   +13498     
- Partials     4870     4886      +16

Files with missing lines	Coverage Δ
internal/core/src/query/SearchOnSealed.cpp	`0.00% <ø> (-100.00%)`	⬇️
internal/datanode/compaction/merge_sort.go	`72.83% <ø> (ø)`
internal/proxy/search_util.go	`82.17% <100.00%> (+0.17%)`	⬆️
internal/proxy/task.go	`89.77% <ø> (ø)`
internal/querynodev2/delegator/delegator.go	`87.42% <100.00%> (+0.28%)`	⬆️
internal/querynodev2/handlers.go	`79.93% <100.00%> (-0.31%)`	⬇️
internal/proxy/task_search.go	`77.41% <91.66%> (+0.46%)`	⬆️
internal/querynodev2/segments/result.go	`71.95% <80.00%> (+4.84%)`	⬆️
internal/querynodev2/segments/search_reduce.go	`86.66% <90.00%> (-1.66%)`	⬇️
internal/util/reduce/reduce_info.go	`95.00% <95.00%> (ø)`
... and 2 more

... and 232 files with indirect coverage changes

Signed-off-by: MrPresent-Han <chun.han@gmail.com>

MrPresent-Han · 2024-09-05T08:19:21Z

internal/proxy/search_reduce_util.go

-	offset              int64
-	queryInfo           *planpb.QueryInfo
+func reduceSearchResult(ctx context.Context, subSearchResultData []*schemapb.SearchResultData, reduceInfo *reduce.ResultInfo) (*milvuspb.SearchResults, error) {
+	if reduceInfo.GetGroupByFieldId() > 0 {


MrPresent-Han · 2024-09-05T08:20:22Z

internal/proxy/search_reduce_util.go

+		}
+	}
+	// reducing nq * topk results
+	for nqIdx := int64(0); nqIdx < nq; nqIdx++ {


when reducing for advance groupby, we just merge results from different delegatror and do not really reduce and sort

MrPresent-Han · 2024-09-05T08:21:24Z

internal/proxy/search_reduce_util.go

+	}
+	return false
+}
+


different score strategy is used here to prepare different scorer

MrPresent-Han · 2024-09-05T08:22:15Z

internal/proxy/search_reduce_util.go

+	return nil
+}
+
+func rankSearchResultDataByPk(ctx context.Context,


for normal hybridSearch, this rank branch is executed

MrPresent-Han · 2024-09-05T08:23:34Z

internal/querynodev2/delegator/delegator.go

@@ -375,12 +381,7 @@ func (sd *shardDelegator) Search(ctx context.Context, req *querypb.SearchRequest
 			}
 			results[i] = result
 		}
-		var ret *internalpb.SearchResults


I removed useless reduce here, and return directly for reduction outside in searchChannel

MrPresent-Han · 2024-09-05T08:25:02Z

internal/querynodev2/services.go

-		result, err2 = segments.ReduceSearchResults(ctx, toReduceResults, segments.NewReduceInfo(req.Req.GetNq(),
-			req.Req.GetTopk(),
-			req.Req.GetExtraSearchParam().GetGroupByFieldId(),
-			req.Req.GetExtraSearchParam().GetGroupSize(),


currently in milvus query framework, only one channel will be searched, queried in one request, which is triggered by proxy, So I simplify code here and remove unnessary reduction between different channels which will never happen at all

MrPresent-Han · 2024-09-05T10:46:16Z

rerun ut

czs007 · 2024-09-06T02:45:24Z

internal/util/reduce/reduce_info.go

+	groupByFieldId int64
+	groupSize      int64
+	isAdvance      bool
+}


add motify options

Signed-off-by: MrPresent-Han <chun.han@gmail.com>

czs007 · 2024-09-08T09:07:10Z

internal/proxy/task_search.go

@@ -403,6 +404,11 @@ func (t *searchTask) initAdvancedSearchRequest(ctx context.Context) error {
 			zap.Int64s("plan.OutputFieldIds", plan.GetOutputFieldIds()),
 			zap.Stringer("plan", plan)) // may be very large if large term passed.
 	}
+	if len(t.queryInfos) > 0 {


to do parse from rankParams

czs007 · 2024-09-08T09:07:48Z

/approve
/lgtm

sre-ci-robot · 2024-09-08T09:08:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: czs007, MrPresent-Han

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:

~~OWNERS~~ [czs007]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

related: milvus-io#35096 Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com>

sre-ci-robot requested review from aoiasd and bigsheeper September 4, 2024 10:53

sre-ci-robot added size/XXL Denotes a PR that changes 1000+ lines. area/internal-api labels Sep 4, 2024

mergify bot added the dco-passed DCO check passed. label Sep 4, 2024

mergify bot added the do-not-merge/invalid-pr-format label Sep 4, 2024

MrPresent-Han force-pushed the support-hybrid-search-groupby-review branch from cf390fc to fe2a9f2 Compare September 4, 2024 12:29

MrPresent-Han pushed a commit to MrPresent-Han/milvus that referenced this pull request Sep 4, 2024

supporing hybrid search group_by(milvus-io#35982)

e05bb3e

Signed-off-by: MrPresent-Han <chun.han@gmail.com>

MrPresent-Han force-pushed the support-hybrid-search-groupby-review branch from fe2a9f2 to e05bb3e Compare September 4, 2024 14:07

MrPresent-Han pushed a commit to MrPresent-Han/milvus that referenced this pull request Sep 5, 2024

supporing hybrid search group_by(milvus-io#35982)

3249e4d

Signed-off-by: MrPresent-Han <chun.han@gmail.com>

MrPresent-Han force-pushed the support-hybrid-search-groupby-review branch from e05bb3e to 3249e4d Compare September 5, 2024 04:15

mergify bot added the ci-passed label Sep 5, 2024

MrPresent-Han pushed a commit to MrPresent-Han/milvus that referenced this pull request Sep 5, 2024

supporing hybrid search group_by(milvus-io#35982)

2ecb24a

Signed-off-by: MrPresent-Han <chun.han@gmail.com>

MrPresent-Han force-pushed the support-hybrid-search-groupby-review branch from 3249e4d to 2ecb24a Compare September 5, 2024 08:13

mergify bot removed the ci-passed label Sep 5, 2024

MrPresent-Han commented Sep 5, 2024

View reviewed changes

mergify bot added the ci-passed label Sep 5, 2024

czs007 reviewed Sep 6, 2024

View reviewed changes

MrPresent-Han pushed a commit to MrPresent-Han/milvus that referenced this pull request Sep 6, 2024

supporing hybrid search group_by(milvus-io#35982)

acf0c3f

Signed-off-by: MrPresent-Han <chun.han@gmail.com>

MrPresent-Han force-pushed the support-hybrid-search-groupby-review branch from 2ecb24a to acf0c3f Compare September 6, 2024 04:03

mergify bot removed the ci-passed label Sep 6, 2024

supporing hybrid search group_by(milvus-io#35982)

4b86fe6

Signed-off-by: MrPresent-Han <chun.han@gmail.com>

MrPresent-Han force-pushed the support-hybrid-search-groupby-review branch from acf0c3f to 4b86fe6 Compare September 6, 2024 06:03

mergify bot added the ci-passed label Sep 6, 2024

czs007 reviewed Sep 8, 2024

View reviewed changes

sre-ci-robot assigned czs007 Sep 8, 2024

sre-ci-robot added the lgtm label Sep 8, 2024

sre-ci-robot added the approved label Sep 8, 2024

czs007 changed the title ~~supporing hybrid search group_by~~ feat: supporing hybrid search group_by Sep 8, 2024

mergify bot added kind/feature Issues related to feature request from users and removed ci-passed do-not-merge/invalid-pr-format labels Sep 8, 2024

sre-ci-robot merged commit e480b10 into milvus-io:master Sep 8, 2024
13 of 16 checks passed

chyezh pushed a commit to chyezh/milvus that referenced this pull request Sep 11, 2024

feat: supporing hybrid search group_by (milvus-io#35982)

dbdefa0

related: milvus-io#35096 Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: supporing hybrid search group_by #35982

feat: supporing hybrid search group_by #35982

MrPresent-Han commented Sep 4, 2024

mergify bot commented Sep 4, 2024

mergify bot commented Sep 4, 2024

mergify bot commented Sep 4, 2024

mergify bot commented Sep 4, 2024

codecov bot commented Sep 5, 2024 •

edited

Loading

MrPresent-Han Sep 5, 2024

MrPresent-Han Sep 5, 2024

MrPresent-Han Sep 5, 2024

MrPresent-Han Sep 5, 2024

MrPresent-Han Sep 5, 2024

MrPresent-Han Sep 5, 2024

MrPresent-Han commented Sep 5, 2024

czs007 Sep 6, 2024

MrPresent-Han Sep 6, 2024

czs007 Sep 8, 2024

czs007 commented Sep 8, 2024

sre-ci-robot commented Sep 8, 2024

feat: supporing hybrid search group_by #35982

feat: supporing hybrid search group_by #35982

Conversation

MrPresent-Han commented Sep 4, 2024

mergify bot commented Sep 4, 2024

mergify bot commented Sep 4, 2024

mergify bot commented Sep 4, 2024

mergify bot commented Sep 4, 2024

codecov bot commented Sep 5, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrPresent-Han commented Sep 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

czs007 commented Sep 8, 2024

sre-ci-robot commented Sep 8, 2024

codecov bot commented Sep 5, 2024 •

edited

Loading