Report QPS about each type of command in RegionHeartbeat and StoreHeartbeat #8157

lhy1024 · 2020-06-29T14:22:13Z

What problem does this PR solve?

Issue Number: close tikv/pd#2526

Problem Summary: Report QPS about each type of command in RegionHeartbeat. for the multi-dimensions hotspot scheduling.

What is changed and how it works?

What's Changed:

Add Grpc info in tls_collect_qps

Related changes

PR to update pingcap/kvproto

Check List

Tests

Unit test
Integration test
Manual test

Release note

Report Read QPS about each type of command in RegionHeartbeat

CLAassistant · 2020-12-16T03:31:27Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ HunDunDM
❌ lhy1024
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

CLAassistant · 2020-12-16T03:31:30Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ HunDunDM
❌ lhy1024
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

lhy1024 · 2021-05-14T06:53:32Z

/run-test

lhy1024 · 2021-05-14T06:53:44Z

/rebuild

Yisaer · 2021-05-14T07:40:29Z

/cc @Yisaer

zhangjinpeng87 · 2021-05-16T12:44:10Z

This PR lasts too long.

zhangjinpeng87 · 2021-05-28T07:57:30Z

Cargo.lock

@@ -2217,7 +2217,7 @@ dependencies = [
 [[package]]
 name = "kvproto"
 version = "0.0.2"
-source = "git+https://github.com/pingcap/kvproto.git?rev=511a3225c29f9272af83d28efed92eba835cd34f#511a3225c29f9272af83d28efed92eba835cd34f"
+source = "git+https://github.com/lhy1024/kvproto.git?rev=881f9799e333c2464c20b647da831ed5c9013d43#881f9799e333c2464c20b647da831ed5c9013d43"


Why lhy1024 branch?

After pingcap/kvproto#638 merged, it will be replaced with pingcap branch.

components/raftstore/src/store/peer.rs

nolouch · 2021-05-29T18:28:13Z

components/raftstore/src/store/fsm/peer.rs

@@ -1114,6 +1114,7 @@ where
                    "peer_id" => self.fsm.peer_id(),
                    "res" => ?res,
                );
+                res.metrics.written_query_num += 1;


It's better in fsm/apply.rs, and the ApplyTaskRes is received after the ApplyPoller call end, it's may handle many commands in one poller round.

Signed-off-by: lhy1024 <admin@liudos.us>

nolouch · 2021-06-03T07:40:32Z

components/raftstore/src/store/worker/pd.rs

@@ -427,6 +437,7 @@ where

 const HOTSPOT_KEY_RATE_THRESHOLD: u64 = 128;
 const HOTSPOT_BYTE_RATE_THRESHOLD: u64 = 8 * 1024;
+const HOTSPOT_QUERY_RATE_THRESHOLD: u64 = 8; // TODO: need more test


I think it's too small, assume the bytes 1 key have 64 bytes, it should be 128.

Signed-off-by: lhy1024 <admin@liudos.us>

nolouch

rest LGTM

nolouch · 2021-06-03T13:19:13Z

components/raftstore/src/store/worker/pd.rs

            region_peer.last_store_report_read_bytes = region_peer.read_bytes;
            region_peer.last_store_report_read_keys = region_peer.read_keys;
+            info!("read peer qps";"region id"=>region_id,"qps"=>read_query_stats.get_read_query_num());


Does this log need to be deleted? it's only for debug?

nolouch · 2021-06-03T13:20:27Z

components/raftstore/src/store/worker/pd.rs

+            .store_stat
+            .engine_total_query_num
+            .sub_query_stats(&self.store_stat.engine_last_query_num);
+        info!("read store qps";"store id"=>stats.get_store_id(),"qps"=>res.get_read_query_num());


due too. or better with metrics.

nolouch · 2021-06-03T13:25:39Z

components/raftstore/src/store/fsm/apply.rs

@@ -1488,6 +1488,7 @@ where
                );
            });
        }
+        self.metrics.written_query_num += 1;


also need for handle_delete

Signed-off-by: lhy1024 <admin@liudos.us>

nolouch

lgtm, plz update kvproto

ti-chi-bot · 2021-06-04T12:38:55Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

Yisaer
nolouch

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Signed-off-by: lhy1024 <admin@liudos.us>

lhy1024 · 2021-06-15T08:04:07Z

/merge

ti-chi-bot · 2021-06-15T08:04:07Z

@lhy1024: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2021-06-15T08:04:09Z

This pull request has been accepted and is ready to merge.

Commit hash: c534edf

zhongzc · 2021-06-21T04:21:17Z

Cargo.toml

+
+[patch.'https://github.com/pingcap/kvproto.git']
+kvproto = { git = "https://github.com/lhy1024/kvproto.git", rev = "dda0a102bc6ac7371e1f99cd2797825aa210b4a0", default-features = false }


Is here on purpose or by accident?

Sorry, an accident.I will fix it later.

zhangjinpeng87 · 2021-07-04T01:52:21Z

components/raftstore/src/store/fsm/apply.rs

@@ -1432,9 +1433,22 @@ where
        for req in requests {
            let cmd_type = req.get_cmd_type();
            let mut resp = match cmd_type {
-                CmdType::Put => self.handle_put(ctx.kv_wb_mut(), req),
-                CmdType::Delete => self.handle_delete(ctx.kv_wb_mut(), req),
+                CmdType::Put => {


Just counting the QPS at apply module can't cover all scenarios, there is a big different CPU cost in grpc and raftstore between 1 request put 100kv and 100 reqeusts with each request puts 1 kv.

emmm, maybe statistics should be placed at a higher level?

lhy1024 added the status/DNM label Jun 29, 2020

lhy1024 requested a review from nolouch June 29, 2020 14:22

lhy1024 self-assigned this Jun 29, 2020

lhy1024 force-pushed the rp branch 2 times, most recently from c8e518f to 0dc9c58 Compare July 1, 2020 17:11

lhy1024 changed the title ~~[DNM] Report Read QPS about each type of command in RegionHeartbeat~~ [DNM] Report QPS about each type of command in RegionHeartbeat Jul 1, 2020

lhy1024 force-pushed the rp branch 2 times, most recently from b8623ab to 989fe0f Compare July 1, 2020 17:30

nolouch requested a review from HunDunDM July 7, 2020 12:04

HunDunDM force-pushed the rp branch from 2768a90 to fb4a433 Compare July 21, 2020 13:16

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 31, 2020

ti-chi-bot removed the status/DNM label Mar 15, 2021

lhy1024 force-pushed the rp branch from 7f80eac to 8380360 Compare May 10, 2021 10:16

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 10, 2021

lhy1024 force-pushed the rp branch from 8380360 to 706ffb4 Compare May 12, 2021 06:34

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 12, 2021

lhy1024 force-pushed the rp branch 2 times, most recently from 6f08ab0 to 00d70b0 Compare May 13, 2021 08:45

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 13, 2021

lhy1024 changed the title ~~[DNM] Report QPS about each type of command in RegionHeartbeat~~ Report QPS about each type of command in RegionHeartbeat and StoreHeartbeat May 13, 2021

ti-chi-bot requested a review from Yisaer May 14, 2021 07:40

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 27, 2021

zhangjinpeng87 reviewed May 28, 2021

View reviewed changes

nolouch reviewed May 29, 2021

View reviewed changes

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 29, 2021

add qps statistic report

acff23d

Signed-off-by: lhy1024 <admin@liudos.us>

lhy1024 force-pushed the rp branch from 88fa5ef to acff23d Compare June 2, 2021 08:47

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 2, 2021

nolouch reviewed Jun 3, 2021

View reviewed changes

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 3, 2021

fix config

cc1896f

Signed-off-by: lhy1024 <admin@liudos.us>

nolouch reviewed Jun 3, 2021

View reviewed changes

update write query by kind

3f03e7c

Signed-off-by: lhy1024 <admin@liudos.us>

nolouch approved these changes Jun 4, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jun 4, 2021

lhy1024 added 3 commits June 15, 2021 15:15

Merge branch 'master' of http://github.com/tikv/tikv into rp

806d367

Signed-off-by: lhy1024 <admin@liudos.us>

update kvproto

8b3800e

Signed-off-by: lhy1024 <admin@liudos.us>

fix lint

c534edf

Signed-off-by: lhy1024 <admin@liudos.us>

ti-chi-bot added status/can-merge Indicates a PR has been approved by a committer. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 15, 2021

ti-chi-bot merged commit 61265d3 into tikv:master Jun 15, 2021

zhongzc reviewed Jun 21, 2021

View reviewed changes

lhy1024 mentioned this pull request Jun 21, 2021

remove unnecessary kvproto patch #10410

Merged

zhangjinpeng87 reviewed Jul 4, 2021

View reviewed changes

nolouch mentioned this pull request Jul 13, 2021

Adjustable Priority Multidimensional Hot Scheduler Tasklist tikv/pd#3869

Closed

28 tasks


		[patch.'https://github.com/pingcap/kvproto.git']
		kvproto = { git = "https://github.com/lhy1024/kvproto.git", rev = "dda0a102bc6ac7371e1f99cd2797825aa210b4a0", default-features = false }

Report QPS about each type of command in RegionHeartbeat and StoreHeartbeat #8157

Report QPS about each type of command in RegionHeartbeat and StoreHeartbeat #8157

Uh oh!

Conversation

lhy1024 commented Jun 29, 2020

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

CLAassistant commented Dec 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Dec 16, 2020

Uh oh!

lhy1024 commented May 14, 2021

Uh oh!

lhy1024 commented May 14, 2021

Uh oh!

Yisaer commented May 14, 2021

Uh oh!

zhangjinpeng87 commented May 16, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nolouch May 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nolouch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nolouch left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot commented Jun 4, 2021

Uh oh!

lhy1024 commented Jun 15, 2021

Uh oh!

ti-chi-bot commented Jun 15, 2021

Uh oh!

ti-chi-bot commented Jun 15, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CLAassistant commented Dec 16, 2020 •

edited

Loading

nolouch May 29, 2021 •

edited

Loading