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

enhance: skip return segment/channel info if no changes happens. #33022

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

enhance: skip return segment/channel info if no changes happens. #33022

wants to merge 2 commits into from

Conversation

weiliu1031
Copy link
Contributor

issue: #32244
querycoord call getRecoveryInfo to get segment/channel list for every 3s, and it will get same content if no changes happens in datacoord, which will cause a meaningless network and cpu cost.

to solve this, we compute md5 signature for segment list, and use this signature to check whether segment list changes, if no changes happens, skip return the segment info in rpc.

@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: weiliu1031
To complete the pull request process, please assign jiaoew1991 after the PR has been reviewed.
You can assign the PR to them by writing /assign @jiaoew1991 in a comment when ready.

The full list of commands accepted by this bot can be found 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 added the size/XL Denotes a PR that changes 500-999 lines. label May 13, 2024
@mergify mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels May 13, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented May 13, 2024

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

@weiliu1031 weiliu1031 force-pushed the datacoord_modify_ts branch 2 times, most recently from caa2cfa to 74c29a7 Compare May 14, 2024 06:15
@codecov Codecov
Copy link

codecov bot commented May 14, 2024 •
edited

Codecov Report

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

Project coverage is 82.09%. Comparing base (a7f6193) to head (74c29a7).
Report is 4 commits behind head on master.

❗ Current head 74c29a7 differs from pull request most recent head bca9662

Please upload reports for the commit bca9662 to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #33022      +/-   ##
==========================================
- Coverage   82.18%   82.09%   -0.09%     
==========================================
  Files        1009     1005       -4     
  Lines      128806   127681    -1125     
==========================================
- Hits       105860   104821    -1039     
+ Misses      18953    18894      -59     
+ Partials     3993     3966      -27
Files Coverage Δ
internal/datacoord/util.go 99.23% <100.00%> (+0.09%) ⬆️
internal/querycoordv2/meta/coordinator_broker.go 92.21% <100.00%> (-0.89%) ⬇️
internal/querycoordv2/observers/target_observer.go 82.11% <100.00%> (+0.93%) ⬆️
internal/querycoordv2/meta/target.go 94.73% <90.00%> (-0.56%) ⬇️
internal/querycoordv2/meta/target_manager.go 88.42% <89.47%> (+0.28%) ⬆️
internal/datacoord/services.go 85.38% <40.00%> (-0.33%) ⬇️

... and 112 files with indirect coverage changes

@mergify mergify bot added the ci-passed label May 14, 2024
@weiliu1031
enhance: skip return segment/channel info if no changes happens.
312d588 
querycoord call getRecoveryInfo to get segment/channel list for every
3s, and it will get same content if no changes happens in datacoord,
which will cause a meaningless network and cpu cost.

to solve this, we compute md5 signature for segment list, and use this
signature to check whether segment list changes, if no changes happens,
skip return the segment info in rpc.

Signed-off-by: Wei Liu <wei.liu@zilliz.com>
@weiliu1031
use murmur3 to compute hash
bca9662 
Signed-off-by: Wei Liu <wei.liu@zilliz.com>
@mergify mergify bot removed the ci-passed label May 20, 2024
@weiliu1031
Copy link
Contributor Author

/hold

@weiliu1031
Copy link
Contributor Author

compute hash for segment id list cost too much cpu

@weiliu1031
Copy link
Contributor Author

rerun ut

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@congqixia congqixia Awaiting requested review from congqixia

@jiaoew1991 jiaoew1991 Awaiting requested review from jiaoew1991

Assignees
No one assigned
Labels
dco-passed DCO check passed. do-not-merge/hold kind/enhancement Issues or changes related to enhancement 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.

None yet
2 participants
@weiliu1031 @sre-ci-robot