*: directly report statistics to pd #2337

nolouch · 2017-09-25T17:33:40Z

refactor coprocessor and report statistics to pd worker

overvenus · 2017-09-26T03:39:35Z

src/pd/metrics.rs

+
+    pub static ref PD_REQ_COUNTER_VEC: CounterVec =
+        register_counter_vec!(
+            "tikv_raftstore_pd_request_sent_total",


Now, it does not belong to tikv_raftstore_ any more.

+1
We should also update the grafana too.

siddontang · 2017-09-27T01:15:50Z

PTAL @BusyJay

siddontang · 2017-09-30T02:56:31Z

Any update @nolouch

siddontang · 2017-10-09T00:44:51Z

src/pd/pd.rs

@@ -395,6 +426,21 @@ impl<T: PdClient> Runner<T> {
        handle.spawn(f);
        self.is_hb_receiver_scheduled = true;
    }
+
+    fn handle_coprocessor_read_stats(&mut self, read_stats: CopRequestStatistics) {


remove the name of coprocessor here, PD should not know it.

siddontang · 2017-10-09T00:45:13Z

src/pd/pd.rs

@@ -444,6 +498,7 @@ impl<T: PdClient> Runnable<Task> for Runner<T> {
            }
            Task::ReportSplit { left, right } => self.handle_report_split(handle, left, right),
            Task::ValidatePeer { region, peer } => self.handle_validate_peer(handle, region, peer),
+            Task::CopReadStats { read_stats } => self.handle_coprocessor_read_stats(read_stats),


ReadStats is fine.

siddontang · 2017-10-09T00:45:38Z

src/coprocessor/endpoint.rs

@@ -63,14 +64,14 @@ const OUTDATED_ERROR_MSG: &'static str = "request outdated.";

 const ENDPOINT_IS_BUSY: &'static str = "endpoint is busy";

-pub struct Host<R: CopSender + 'static> {
+pub struct Host {


use another PR for refactoring coprocessor.

siddontang · 2017-10-09T00:48:59Z

src/coprocessor/endpoint.rs

@@ -800,7 +792,8 @@ mod tests {
        let engine = engine::new_local_engine(TEMP_DIR, &[]).unwrap();
        let mut cfg = Config::default();
        cfg.end_point_concurrency = 1;
-        let mut end_point = Host::new(engine, worker.scheduler(), &cfg, MockCopSender::new());
+        let pd_worker = FutureWorker::new("test-pd-worker");


how to check we have already sent the read statistics?

maybe mock a pd runner?

seem no need now.

siddontang · 2017-10-10T06:43:26Z

src/raftstore/store/metrics.rs

@@ -129,20 +129,6 @@ lazy_static! {
             exponential_buckets(1.0, 2.0, 20).unwrap()
        ).unwrap();

-    pub static ref REGION_READ_KEYS_HISTOGRAM: Histogram =


no this metric now?

siddontang · 2017-10-11T01:32:06Z

src/pd/pd.rs

+        self.store_stat.engine_last_total_bytes_read = self.store_stat.engine_total_bytes_read;
+        self.store_stat.engine_last_total_keys_read = self.store_stat.engine_total_keys_read;
+
+        for peer in self.region_peers.values() {


define a local metric and flush after the for loop like https://github.com/pingcap/tikv/blob/master/src/raftstore/store/store.rs#L1692

I prefer handling write bytes/keys to PD worker too.

siddontang · 2017-10-11T09:40:58Z

Any update @nolouch

siddontang · 2017-10-11T13:40:34Z

Cargo.toml

@@ -80,7 +80,7 @@ git = "https://github.com/pingcap/kvproto.git"
 git = "https://github.com/pingcap/tipb.git"

 [dependencies.prometheus]
-version = "0.3"
+git = "https://github.com/pingcap/rust-prometheus.git"


@overvenus

Please update the crate.

siddontang · 2017-10-12T02:03:55Z

src/raftstore/store/store.rs

@@ -1693,34 +1676,13 @@ impl<T: Transport, C: PdClient> Store<T, C> {
        for peer in self.region_peers.values_mut() {
            peer.peer_stat.last_written_bytes = peer.peer_stat.written_bytes;


can we save the last_written_bytes/keys in PD worker and update in the heartbeat msg?

nolouch · 2017-10-12T06:44:34Z

PTAL

siddontang · 2017-10-12T07:43:45Z

src/pd/pd.rs

+                    let peer_stat = self.region_peers
+                        .entry(region.get_id())
+                        .or_insert_with(PeerStat::default);
+                    let read_bytes_interval = peer_stat.read_bytes - peer_stat.last_read_bytes;


delta may be better than interval

siddontang · 2017-10-12T07:44:30Z

Rest LGTM

PTAL @huachaohuang @zhangjinpeng1987

zhangjinpeng87 · 2017-10-13T02:04:04Z

src/pd/pd.rs

@@ -114,6 +157,8 @@ pub struct Runner<T: PdClient> {
    pd_client: Arc<T>,
    ch: SendCh<Msg>,
    db: Arc<DB>,
+    region_peers: HashMap<u64, PeerStat>,


When a peer is dropped in raftstore, here should keep updated.

zhangjinpeng87

LGTM

move out pd work

0618283

nolouch requested review from siddontang, disksing and zhangjinpeng87 September 25, 2017 17:33

remove whitespace

2606c8b

overvenus reviewed Sep 26, 2017

View reviewed changes

nolouch changed the title *: move out pd work from raftstore [WIP] *: move out pd work from raftstore Sep 26, 2017

Merge branch 'master' into shuning/extract-pd

893a9ca

nolouch added 4 commits September 30, 2017 18:01

pick up read report

7df14b3

fix test

8697cad

merge master

02b2003

clean up

4ae5719

nolouch changed the title ~~[WIP] *: move out pd work from raftstore~~ *: move out pd work from raftstore Oct 8, 2017

siddontang reviewed Oct 9, 2017

View reviewed changes

nolouch changed the title *: move out pd work from raftstore *: directly report statistics to pd Oct 9, 2017

nolouch mentioned this pull request Oct 9, 2017

*: move out pd work from raftstore #2361

Merged

nolouch added 3 commits October 9, 2017 13:42

address comment

33b6a32

--wip-- [skip ci]

60ec974

fix conflict

d61b223

siddontang reviewed Oct 10, 2017

View reviewed changes

nolouch added 2 commits October 10, 2017 18:42

fix metrics

4901ca8

address comment

7398ac3

siddontang reviewed Oct 11, 2017

View reviewed changes

address comment

f9e04eb

nolouch added 2 commits October 11, 2017 20:23

merge master

3d9c332

fix conflict

8de7261

siddontang reviewed Oct 11, 2017

View reviewed changes

nolouch added 2 commits October 11, 2017 22:11

change to version

aad67c2

tiny cleanup

06a33c0

siddontang reviewed Oct 12, 2017

View reviewed changes

nolouch added 3 commits October 12, 2017 11:43

address comment

5e4e30b

cleanup

4cb0dc9

fix conflict

cd69857

siddontang reviewed Oct 12, 2017

View reviewed changes

address comment

5d2e6f8

zhangjinpeng87 reviewed Oct 13, 2017

View reviewed changes

address comment

9a2705a

zhangjinpeng87 approved these changes Oct 13, 2017

View reviewed changes

nolouch merged commit 18beaa3 into master Oct 13, 2017

nolouch deleted the shuning/extract-pd-work branch October 13, 2017 07:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: directly report statistics to pd #2337

*: directly report statistics to pd #2337

nolouch commented Sep 25, 2017 •

edited

Loading

overvenus Sep 26, 2017

siddontang Sep 30, 2017

siddontang commented Sep 27, 2017

siddontang commented Sep 30, 2017

siddontang Oct 9, 2017

siddontang Oct 9, 2017

siddontang Oct 9, 2017

siddontang Oct 9, 2017

nolouch Oct 9, 2017

siddontang Oct 10, 2017

siddontang Oct 10, 2017

siddontang Oct 11, 2017

siddontang Oct 11, 2017

siddontang commented Oct 11, 2017

siddontang Oct 11, 2017

siddontang Oct 12, 2017

nolouch commented Oct 12, 2017

siddontang Oct 12, 2017

siddontang commented Oct 12, 2017

zhangjinpeng87 Oct 13, 2017

zhangjinpeng87 left a comment

		@@ -1693,34 +1676,13 @@ impl<T: Transport, C: PdClient> Store<T, C> {
		for peer in self.region_peers.values_mut() {
		peer.peer_stat.last_written_bytes = peer.peer_stat.written_bytes;

*: directly report statistics to pd #2337

*: directly report statistics to pd #2337

Conversation

nolouch commented Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siddontang commented Sep 27, 2017

siddontang commented Sep 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siddontang commented Oct 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolouch commented Oct 12, 2017

Choose a reason for hiding this comment

siddontang commented Oct 12, 2017

Choose a reason for hiding this comment

zhangjinpeng87 left a comment

Choose a reason for hiding this comment

nolouch commented Sep 25, 2017 •

edited

Loading