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.

Sign up for GitHub

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

Jump to bottom

Improve load performance #16514

Merged
merged 1 commit into from
Apr 25, 2022
Merged

Improve load performance #16514

merged 1 commit into from
Apr 25, 2022

Conversation

yah01
Copy link
Member

@yah01 yah01 commented Apr 18, 2022 •
edited
Loading

Signed-off-by: yah01 yang.cen@zilliz.com
#15736 #13264
/kind improvement

previous closed pr: #16061

Now the segment loader loads the segment's fields concurrently, which helps reduce memory usage during loading. Further optimization for reducing memory usage is coming.

@sre-ci-robot sre-ci-robot added kind/improvement Changes related to something improve, likes ut and code refactor area/dependency Pull requests that update a dependency file labels Apr 18, 2022
@sre-ci-robot sre-ci-robot added the size/XL Denotes a PR that changes 500-999 lines. label Apr 18, 2022
@mergify mergify bot added the dco-passed DCO check passed. label Apr 18, 2022
@yah01
Copy link
Member Author

yah01 commented Apr 18, 2022

/assign @congqixia
/assign @xiaofan-luan

@codecov
Copy link

codecov bot commented Apr 18, 2022 •
edited
Loading

Codecov Report

Merging #16514 (1298fad) into master (b99b65c) will increase coverage by 0.01%.
The diff coverage is 80.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16514      +/-   ##
==========================================
+ Coverage   80.72%   80.74%   +0.01%     
==========================================
  Files         437      439       +2     
  Lines       69113    69351     +238     
==========================================
+ Hits        55790    55995     +205     
- Misses      10670    10687      +17     
- Partials     2653     2669      +16
Impacted Files Coverage Δ
internal/util/concurrency/pool.go 62.96% <62.96%> (ø)
internal/querynode/segment_loader.go 68.42% <76.47%> (-0.72%) ⬇️
internal/util/funcutil/parallel.go 85.06% <88.37%> (-14.94%) ⬇️
internal/util/concurrency/future.go 89.28% <89.28%> (ø)
internal/storage/data_codec.go 67.65% <100.00%> (+0.03%) ⬆️
internal/datacoord/segment_manager.go 82.03% <0.00%> (-1.02%) ⬇️
internal/log/zap_text_encoder.go 83.59% <0.00%> (+0.44%) ⬆️
internal/distributed/rootcoord/service.go 81.64% <0.00%> (+0.78%) ⬆️
... and 8 more

@yah01 yah01 force-pushed the improve_load_perf branch from c0a9e7d to 0f8c90b Compare April 18, 2022 12:59
@mergify
Copy link
Contributor

mergify bot commented Apr 18, 2022

@yah01 ut workflow job failed, comment rerun ut can trigger the job again.

@yah01 yah01 force-pushed the improve_load_perf branch 2 times, most recently from 84a1f8d to cc62947 Compare April 19, 2022 04:03
@mergify mergify bot added the ci-passed label Apr 19, 2022
@yah01 yah01 force-pushed the improve_load_perf branch from cc62947 to 444ab4b Compare April 21, 2022 05:24
@sre-ci-robot sre-ci-robot added size/L Denotes a PR that changes 100-499 lines. and removed size/XL Denotes a PR that changes 500-999 lines. labels Apr 21, 2022
@mergify mergify bot removed the ci-passed label Apr 21, 2022
@yah01 yah01 force-pushed the improve_load_perf branch from 444ab4b to 5c8b179 Compare April 21, 2022 06:55
@sre-ci-robot sre-ci-robot added size/XL Denotes a PR that changes 500-999 lines. and removed size/L Denotes a PR that changes 100-499 lines. labels Apr 21, 2022
@mergify
Copy link
Contributor

mergify bot commented Apr 21, 2022

@yah01 E2e jenkins job failed, comment /run-checks can trigger the job again.

@mergify
Copy link
Contributor

mergify bot commented Apr 21, 2022

@yah01 ut workflow job failed, comment rerun ut can trigger the job again.

@yah01 yah01 force-pushed the improve_load_perf branch from 5c8b179 to ce768d1 Compare April 21, 2022 08:03
@mergify
Copy link
Contributor

mergify bot commented Apr 21, 2022

@yah01 E2e jenkins job failed, comment /run-checks can trigger the job again.

@mergify
Copy link
Contributor

mergify bot commented Apr 21, 2022

@yah01 ut workflow job failed, comment rerun ut can trigger the job again.

@yah01 yah01 force-pushed the improve_load_perf branch from ce768d1 to 7a2c448 Compare April 21, 2022 08:31
@mergify
Copy link
Contributor

mergify bot commented Apr 21, 2022

@yah01 E2e jenkins job failed, comment /run-checks can trigger the job again.

@yah01 yah01 force-pushed the improve_load_perf branch from 7a2c448 to bb617c5 Compare April 21, 2022 09:55
@mergify
Copy link
Contributor

mergify bot commented Apr 21, 2022

@yah01 E2e jenkins job failed, comment /run-checks can trigger the job again.

@mergify
Copy link
Contributor

mergify bot commented Apr 21, 2022

@yah01 ut workflow job failed, comment rerun ut can trigger the job again.

@yah01 yah01 force-pushed the improve_load_perf branch from bb617c5 to 1021bd7 Compare April 22, 2022 04:36
@mergify
Copy link
Contributor

mergify bot commented Apr 22, 2022

@yah01 E2e jenkins job failed, comment /run-checks can trigger the job again.

@yah01
Copy link
Member Author

yah01 commented Apr 22, 2022

/run-checks

@mergify
Copy link
Contributor

mergify bot commented Apr 22, 2022

@yah01 E2e jenkins job failed, comment /run-checks can trigger the job again.

1 similar comment
@mergify
Copy link
Contributor

mergify bot commented Apr 22, 2022

@yah01 E2e jenkins job failed, comment /run-checks can trigger the job again.

@mergify mergify bot added the ci-passed label Apr 22, 2022
@yah01
Improve load performance, load binlogs concurrently per file, deseria…
1298fad 
…lize binlogs concurrently per field/segment

Signed-off-by: yah01 <yang.cen@zilliz.com>
@yah01 yah01 force-pushed the improve_load_perf branch from 1021bd7 to 1298fad Compare April 25, 2022 05:35
@mergify mergify bot added ci-passed and removed ci-passed labels Apr 25, 2022
congqixia
congqixia reviewed Apr 25, 2022
View reviewed changes
internal/util/concurrency/future.go
@@ -0,0 +1,51 @@
package concurrency
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: concurrency package need more comments.

internal/util/funcutil/parallel.go
Comment on lines +167 to +174
// if option.preExecute != nil {
// err := option.preExecute()
// if err != nil {
// close(quit)
// wg.Wait()
// return err
// }
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

option.preExecute/ postExecute comment can be deleted.

congqixia
congqixia approved these changes Apr 25, 2022
View reviewed changes
Copy link
Contributor

@congqixia congqixia left a comment

Choose a reason for hiding this comment

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

/lgtm

@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: congqixia, yah01

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot merged commit 7af02fa into milvus-io:master Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiaofan-luan xiaofan-luan xiaofan-luan left review comments

@congqixia congqixia congqixia approved these changes

@bigsheeper bigsheeper Awaiting requested review from bigsheeper

@longjiquan longjiquan Awaiting requested review from longjiquan

Assignees

@xiaofan-luan xiaofan-luan

@congqixia congqixia

Labels
approved area/dependency Pull requests that update a dependency file ci-passed dco-passed DCO check passed. kind/improvement Changes related to something improve, likes ut and code refactor lgtm size/XL Denotes a PR that changes 500-999 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yah01 @sre-ci-robot @xiaofan-luan @congqixia