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

raftstore: make batch strategy more fair #4200

Merged
merged 14 commits into from Feb 18, 2019

Conversation

@hicqu
Copy link
Contributor

commented Feb 12, 2019

What have you changed? (mandatory)

Make the batch strategy more fair.
Previously the batch system won't fetch more readies from other regions beside those currently in processing. So if there are many regions, and the write skews on just some regions, the other regions could be hungry.
This PR changes this behavior. Now the batch system can fetch messages from other regions, even if the current processing set is not finished.

What are the type of the changes? (mandatory)

Improvement.

How has this PR been tested? (mandatory)

CI.

Benchmark result if necessary (optional)

old result:
default
new result:
default

raftstore: make batch strategy more fair.
Signed-off-by: qupeng <qupeng@pingcap.com>

@hicqu hicqu force-pushed the hicqu:raft-batch-fix branch from d0dc5eb to 4cb247b Feb 12, 2019

@hicqu hicqu requested review from BusyJay and overvenus Feb 12, 2019

Show resolved Hide resolved src/raftstore/store/fsm/batch.rs Outdated
Show resolved Hide resolved src/raftstore/store/fsm/metrics.rs Outdated
@BusyJay

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2019

How does it perform under regular workload?

@hicqu

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2019

For the normal cases (not hot point), the benchmark result of master branch:
default
And the result of the patch:
default
The workload contains 50% select and 50% update.
They are almost same.

address comments.
Signed-off-by: qupeng <qupeng@pingcap.com>
@BusyJay

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2019

What about pure updates? Selection is usually handled by local read thread, so it should not be affected too much.

hicqu added some commits Feb 13, 2019

address comments.
Signed-off-by: qupeng <qupeng@pingcap.com>
Remove a useless metrics.
Signed-off-by: qupeng <qupeng@pingcap.com>

hicqu added some commits Feb 14, 2019

address comments.
Signed-off-by: qupeng <qupeng@pingcap.com>
format code.
Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu

This comment has been minimized.

Copy link
Contributor Author

commented Feb 14, 2019

@BusyJay for pure update workload, the result shows this PR won't introduce any QPS dropping.
The left is for the patch, and the right is for master.
default

@hicqu

This comment has been minimized.

Copy link
Contributor Author

commented Feb 14, 2019

@zhangjinpeng1987

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

I think we should add a simulating test to bench the batch system, maybe another PR.
BTW, is it possible to use unit tests to discover such hungry problems?

zhangjinpeng1987 and others added some commits Feb 15, 2019

address comments.
Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2019

PTAL @BusyJay @zhangjinpeng1987 thank you!

@zhangjinpeng1987
Copy link
Member

left a comment

LGTM

@zhangjinpeng1987

This comment has been minimized.

Copy link
Member

commented Feb 16, 2019

@BusyJay Any problem for this new strategy?

@hicqu hicqu merged commit 46f16fa into tikv:master Feb 18, 2019

2 checks passed

DCO All commits are signed off!
Details
idc-jenkins-ci/test Jenkins job succeeded.
Details

@hicqu hicqu deleted the hicqu:raft-batch-fix branch Feb 18, 2019

dcalvin added a commit to dcalvin/tikv that referenced this pull request Feb 22, 2019

raftstore: make batch strategy more fair (tikv#4200)
Signed-off-by: qupeng <qupeng@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.