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

Add kvstore rate limiter #2557

Merged
merged 4 commits into from
Aug 26, 2021
Merged

Add kvstore rate limiter #2557

merged 4 commits into from
Aug 26, 2021

Conversation

critical27
Copy link
Contributor

@critical27 critical27 commented Aug 24, 2021

Add a simple rate limiter, there are mainly two major cases:

  1. pull snapshot when balance or catch up. (limited to 2Mb * 4 part by default )
  2. rebuild index (limited to 1Mb * 10 subtasks by default)

pulling snapshot
image

rebuild index, note that both recv/send will be around 10Mb because all storages is rebuilding
image

@CLAassistant
Copy link

CLAassistant commented Aug 24, 2021

CLA assistant check
All committers have signed the CLA.

@critical27 critical27 force-pushed the rate branch 2 times, most recently from bb176da to 0ed0962 Compare August 24, 2021 04:06
}

void consume(GraphSpaceID spaceId, PartitionID partId, size_t toConsume) {
DCHECK(buckets_.find({spaceId, partId}) != buckets_.end());
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, just to prevent misuse.

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't it need to be locked here?

Copy link
Contributor Author

@critical27 critical27 Aug 26, 2021

Choose a reason for hiding this comment

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

There won't be concurrent add/remove/consume of a {spaceId, partId}. Caller must make sure part has been added, and won't remove during consume. I'll add some comments.

@bright-starry-sky
Copy link
Contributor

Good job , LGTM .

panda-sheep
panda-sheep previously approved these changes Aug 25, 2021
Copy link
Contributor

@panda-sheep panda-sheep left a comment

Choose a reason for hiding this comment

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

Good job!

@yixinglu yixinglu added the doc affected PR: improvements or additions to documentation label Aug 26, 2021
@critical27 critical27 merged commit d886310 into vesoft-inc:master Aug 26, 2021
@critical27 critical27 deleted the rate branch August 26, 2021 12:53
critical27 added a commit to critical27/nebula that referenced this pull request Aug 31, 2021
bright-starry-sky pushed a commit that referenced this pull request Aug 31, 2021
* get meta version in hb

* cherry-pick #549 #550 #551

* damn timeout

* fix rebuild index bug introduced in #2557
yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Mar 21, 2022
* get meta version in hb

* cherry-pick vesoft-inc#549 vesoft-inc#550 vesoft-inc#551

* damn timeout

* fix rebuild index bug introduced in vesoft-inc#2557

Co-authored-by: Doodle <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc affected PR: improvements or additions to documentation ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants