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

enhance: optimize datanode cpu usage under large collection number #33267

Merged
merged 1 commit into from
May 24, 2024

Conversation

xiaofan-luan
Copy link
Contributor

fix #33266
try to improve cpu usage by refactoring the ttchecker logic and caching string

@sre-ci-robot sre-ci-robot added the size/L Denotes a PR that changes 100-499 lines. label May 22, 2024
@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xiaofan-luan

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

@mergify mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels May 22, 2024
Copy link
Contributor

mergify bot commented May 22, 2024

@xiaofan-luan ut workflow job failed, comment rerun ut can trigger the job again.

Copy link
Contributor

mergify bot commented May 22, 2024

@xiaofan-luan ut workflow job failed, comment rerun ut can trigger the job again.

Copy link

codecov bot commented May 23, 2024

Codecov Report

Attention: Patch coverage is 91.80328% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 82.04%. Comparing base (e1bafd7) to head (a8fe9a9).
Report is 29 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #33267      +/-   ##
==========================================
- Coverage   82.20%   82.04%   -0.17%     
==========================================
  Files        1009     1012       +3     
  Lines      128855   128858       +3     
==========================================
- Hits       105927   105718     -209     
- Misses      18944    19159     +215     
+ Partials     3984     3981       -3     
Files Coverage Δ
internal/datanode/timetick_sender.go 100.00% <100.00%> (ø)
internal/datanode/writebuffer/write_buffer.go 92.03% <100.00%> (ø)
internal/util/flowgraph/input_node.go 85.14% <100.00%> (+0.30%) ⬆️
internal/util/flowgraph/node.go 87.12% <100.00%> (ø)
internal/util/pipeline/node.go 100.00% <100.00%> (+14.28%) ⬆️
pkg/util/timerecord/group_checker.go 100.00% <100.00%> (ø)
internal/util/pipeline/pipeline.go 78.78% <84.61%> (-0.63%) ⬇️
internal/datanode/flow_graph_dd_node.go 92.54% <72.72%> (-0.50%) ⬇️

... and 72 files with indirect coverage changes

Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
func (gc *GroupChecker) Check(name string) {
gc.lastest.Insert(name, time.Now())
func (gc *CheckerManager) Register(name string, checker *Checker) {
gc.checkers.Insert(name, checker)
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we always create a new one. Should we check if name already exists?

@yiwangdr
Copy link
Contributor

/lgtm

@sre-ci-robot sre-ci-robot merged commit 36cbce4 into milvus-io:master May 24, 2024
15 checks passed
@czs007 czs007 added the PR | need cherry-pick need cherry pick to other branches label Jun 26, 2024
@mergify mergify bot removed the ci-passed label Jun 26, 2024
bigsheeper pushed a commit to bigsheeper/milvus that referenced this pull request Jul 3, 2024
jaime0815 added a commit to jaime0815/milvus that referenced this pull request Jul 3, 2024
…ilvus-io#33267)

fix milvus-io#33266
try to improve cpu usage by refactoring the ttchecker logic and caching
string

Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>

(cherry picked from commit 36cbce4)
Co-authored-by:: jaime <yun.zhang@zilliz.com>
jaime0815 added a commit to jaime0815/milvus that referenced this pull request Jul 3, 2024
…ilvus-io#33267)

fix milvus-io#33266
try to improve cpu usage by refactoring the ttchecker logic and caching
string

Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>

(cherry picked from commit 36cbce4)
Co-authored-by:: jaime <yun.zhang@zilliz.com>
jaime0815 added a commit to jaime0815/milvus that referenced this pull request Jul 3, 2024
…ilvus-io#33267)

fix milvus-io#33266
try to improve cpu usage by refactoring the ttchecker logic and caching
string

Co-authored-by: xiaofanluan <xiaofan.luan@zilliz.com>

(cherry picked from commit 36cbce4)
Signed-off-by: jaime <yun.zhang@zilliz.com>
jaime0815 added a commit to jaime0815/milvus that referenced this pull request Jul 3, 2024
…ilvus-io#33267)

fix milvus-io#33266
try to improve cpu usage by refactoring the ttchecker logic and caching
string

Co-authored-by: yiwangdr <yiwangdr@gmail.com>

(cherry picked from commit 36cbce4)
Signed-off-by: jaime <yun.zhang@zilliz.com>
@jaime0815 jaime0815 removed the PR | need cherry-pick need cherry pick to other branches label Jul 3, 2024
@mergify mergify bot added the ci-passed label Jul 3, 2024
sre-ci-robot pushed a commit that referenced this pull request Jul 4, 2024
…date logic of ttchecker (#34383)

This PR cherry-picks the following commits:
- Try to improve cpu usage by refactoring the ttchecker logic and
caching string. #33267
- Correct the update logic of timerecorder in the flowgraph to avoid
false failure: "some node(s) haven't received input".
#34339

issue: #33266,
#34337

pr: #33267,
#34339

---------

Signed-off-by: bigsheeper <yihao.dai@zilliz.com>
Co-authored-by: Xiaofan <83447078+xiaofan-luan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved ci-passed dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement lgtm size/L Denotes a PR that changes 100-499 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement]: Optimize datanode cpu usage under large collection number
5 participants