raftstore: Remove future_poller pool and batch Ticks #8457

Little-Wallace · 2020-08-17T08:45:10Z

Signed-off-by: Little-Wallace bupt2013211450@gmail.com

What problem does this PR solve?

To reduce the cost of TiKV heartbeat tick.

What is changed and how it works?

Batch all ticks in once poll together and call them in delay thread rather than future-poller.

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
PR to update pingcap/tidb-ansible:
Need to cherry-pick to the release branch

Check List

Tests

Integration test

Side effects

Performance regression
- Consumes more CPU
- Consumes more MEM
Breaking backward compatibility

Release note

No release note.

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace · 2020-08-17T08:47:53Z

I tested it in a large cluster which has 25K regions. There is result ( I update cluster binary in 13:40):

gengliqi · 2020-08-19T12:41:13Z

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

@@ -691,6 +707,37 @@ impl<EK: KvEngine, ER: RaftEngine, T: Transport, C: PdClient> RaftPoller<EK, ER,
            self.poll_ctx.raft_metrics.ready.snapshot - self.previous_metrics.ready.snapshot
        );
    }
+
+    fn flush_ticks(&mut self) {
+        const TICKS: &[PeerTicks] = &[


Better use static and add a test to check if all types of PeerTicks has been included.

gengliqi · 2020-08-20T04:45:50Z

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

+        });
+        let idx = tick.bits() as usize;
+        let batch = &mut self.ctx.tick_batch[idx];
+        if batch.ticks.is_empty() || batch.wait_duration == timeout {


Maybe we can batch all of the timer and combine the same timeout timer.

But the timeout of most ticks are different...

BusyJay · 2020-08-20T05:16:43Z

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

@@ -691,6 +707,37 @@ impl<EK: KvEngine, ER: RaftEngine, T: Transport, C: PdClient> RaftPoller<EK, ER,
            self.poll_ctx.raft_metrics.ready.snapshot - self.previous_metrics.ready.snapshot
        );
    }
+
+    fn flush_ticks(&mut self) {
+        const TICKS: &[PeerTicks] = &[


How about adding a method to PeerTicks to return all ticks?

NingLin-P · 2020-08-27T07:03:51Z

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

+#[derive(Default)]
+pub struct PeerTickBatch {
+    pub ticks: Vec<Box<dyn FnOnce() + Send>>,
+    pub wait_duration: Duration,


Maybe we can remove it, for the same tick the timeout should be the same, we can get the timeout from config when flushing ticks.

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace · 2020-08-30T17:04:55Z

/test

NingLin-P · 2020-08-31T06:40:27Z

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

@@ -374,6 +390,21 @@ where
        }
        timeout
    }
+
+    pub fn update_ticks_timeout(&mut self) {


Should call this function after config updated at

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

Lines 727 to 747 in 7ce49e7

// update config

self.poll_ctx.perf_context_statistics.start();

if let Some(incoming) = self.cfg_tracker.any_new() {

match Ord::cmp(

&incoming.messages_per_tick,

&self.poll_ctx.cfg.messages_per_tick,

) {

CmpOrdering::Greater => {

self.store_msg_buf.reserve(incoming.messages_per_tick);

self.peer_msg_buf.reserve(incoming.messages_per_tick);

self.messages_per_tick = incoming.messages_per_tick;

}

CmpOrdering::Less => {

self.store_msg_buf.shrink_to(incoming.messages_per_tick);

self.peer_msg_buf.shrink_to(incoming.messages_per_tick);

self.messages_per_tick = incoming.messages_per_tick;

}

_ => {}

}

self.poll_ctx.cfg = incoming.clone();

}

or instead of maintaining wait_duration, we can get the up-to-date config from self.cfg through a function like fn get_wait_duration(&self, tick: PeerTicks) -> Duration

codecov · 2020-08-31T07:42:31Z

Codecov Report

Merging #8457 into master will increase coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8457      +/-   ##
==========================================
+ Coverage   84.45%   84.55%   +0.09%     
==========================================
  Files         596      590       -6     
  Lines      145303   143853    -1450     
==========================================
- Hits       122714   121629    -1085     
+ Misses      22589    22224     -365

Impacted Files	Coverage Δ
components/pd_client/src/errors.rs	`0.00% <0.00%> (-38.47%)`	⬇️
components/error_code/src/raftstore.rs	`0.00% <0.00%> (-21.06%)`	⬇️
components/sst_importer/src/errors.rs	`28.26% <0.00%> (-19.57%)`	⬇️
components/engine_traits/src/errors.rs	`0.00% <0.00%> (-18.19%)`	⬇️
components/pd_client/src/client.rs	`71.42% <0.00%> (-17.49%)`	⬇️
components/txn_types/src/lib.rs	`21.62% <0.00%> (-13.52%)`	⬇️
components/raftstore/src/router.rs	`78.49% <0.00%> (-11.41%)`	⬇️
components/test_raftstore/src/router.rs	`55.55% <0.00%> (-7.31%)`	⬇️
src/storage/txn/mod.rs	`28.57% <0.00%> (-6.50%)`	⬇️
src/storage/kv/cursor.rs	`73.44% <0.00%> (-5.44%)`	⬇️
... and 157 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5496d07...2bd9257. Read the comment docs.

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

…beat

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

NingLin-P

LGTM

ti-srebot · 2020-09-07T07:10:34Z

@NingLin-P,Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments.See the corresponding SIG page for more information. Related SIG: raft(slack).

Little-Wallace · 2020-09-07T09:14:56Z

/test

Little-Wallace · 2020-09-07T10:15:58Z

/test

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

gengliqi · 2020-09-09T07:33:10Z

tests/failpoints/cases/test_conf_change.rs

@@ -158,8 +158,6 @@ fn test_tick_after_destroy() {
    let engine_3 = cluster.get_engine(3);
    must_get_equal(&engine_3, b"k1", b"v1");

-    let tick_fp = "on_raft_log_gc_tick_1";
-    fail::cfg(tick_fp, "pause").unwrap();


Change this failpoint to function on_raft_gc_log_tick and use return not pause then set raft_log_gc_tick_interval to 50ms in this test.

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

…beat

gengliqi · 2020-09-09T09:36:38Z

tests/failpoints/cases/test_conf_change.rs

@@ -176,9 +176,9 @@ fn test_tick_after_destroy() {
    cluster.must_put(b"k3", b"v3");

    thread::sleep(cluster.cfg.raft_store.raft_log_gc_tick_interval.0);
-    fail::remove(tick_fp);


The order should not be changed.

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

gengliqi

LGTM

ti-srebot · 2020-09-09T14:58:15Z

@Little-Wallace Oops! auto merge is restricted to Committers of the SIG.See the corresponding SIG page for more information. Related SIG: raft(slack).

gengliqi · 2020-09-09T14:59:20Z

/merge

ti-srebot · 2020-09-09T14:59:23Z

@gengliqi Oops! This PR requires at least 2 LGTMs to merge. The current number of LGTM is 1.

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot · 2020-09-09T15:26:58Z

cherry pick to release-4.0 in PR #8634

* cherry pick #8457 to release-4.0 Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

remove future_poller pool

051e5aa

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace changed the title ~~Remove future_poller pool and batch Ticks~~ raftstore: Remove future_poller pool and batch Ticks Aug 17, 2020

Little-Wallace added the sig/raft Component: Raft, RaftStore, etc. label Aug 17, 2020

Little-Wallace requested a review from BusyJay August 19, 2020 08:10

gengliqi reviewed Aug 20, 2020

View reviewed changes

BusyJay reviewed Aug 20, 2020

View reviewed changes

BusyJay requested a review from NingLin-P August 26, 2020 07:29

NingLin-P reviewed Aug 27, 2020

View reviewed changes

Little-Wallace added 3 commits August 28, 2020 11:12

Merge branch 'master' into heartbeat

4825336

fix conflict

14d6cb2

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

refactor ticks wait duration

f6f9e2c

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace added needs-cherry-pick-release-4.0 Type: Need cherry pick to release 4.0 needs-cherry-pick-release-3.0 Type: Need cherry pick to release 3.0 labels Aug 29, 2020

Little-Wallace added 2 commits August 29, 2020 11:54

Merge branch 'master' into heartbeat

2a4947f

fix conflict

142ec04

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

NingLin-P reviewed Aug 31, 2020

View reviewed changes

update cfg by dynamic for tick

e25346d

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

hunterlxt mentioned this pull request Sep 1, 2020

Upgrade future for TiKV components #8538

Merged

Little-Wallace added 5 commits September 7, 2020 12:48

Merge branch 'master' into heartbeat

d542cc1

Merge branch 'heartbeat' of github.com:Little-Wallace/tikv into heart…

e0e2334

…beat

fix tmp

be2a0e2

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

use futures03 to notify tick

1381c54

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

reset batch test

a15bf28

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

NingLin-P approved these changes Sep 7, 2020

View reviewed changes

Little-Wallace added 4 commits September 7, 2020 20:48

fix unstable test

1892bb2

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Merge branch 'master' into heartbeat

cacee1a

Merge branch 'master' into heartbeat

ca72fc0

fix fmt because merge

679a23d

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace removed the needs-cherry-pick-release-3.0 Type: Need cherry pick to release 3.0 label Sep 9, 2020

Merge branch 'master' into heartbeat

5eda7bf

gengliqi reviewed Sep 9, 2020

View reviewed changes

Little-Wallace added 2 commits September 9, 2020 17:04

fix gc log tick

1b9285f

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Merge branch 'heartbeat' of github.com:Little-Wallace/tikv into heart…

f249810

…beat

gengliqi reviewed Sep 9, 2020

View reviewed changes

Little-Wallace force-pushed the heartbeat branch from d3d2b1e to 2760123 Compare September 9, 2020 13:53

fix conf change

2760123

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

gengliqi approved these changes Sep 9, 2020

View reviewed changes

ti-srebot added the status/LGT1 Status: PR - There is already 1 approval label Sep 9, 2020

Merge branch 'master' into heartbeat

7481ea0

Little-Wallace added the status/can-merge Status: Can merge to base branch label Sep 9, 2020

Merge branch 'master' into heartbeat

2bd9257

gengliqi merged commit 03c3ec6 into tikv:master Sep 9, 2020

ti-srebot pushed a commit to ti-srebot/tikv that referenced this pull request Sep 9, 2020

cherry pick tikv#8457 to release-4.0

1691c65

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot mentioned this pull request Sep 9, 2020

raftstore: Remove future_poller pool and batch Ticks (#8457) #8634

Merged

Little-Wallace pushed a commit that referenced this pull request Sep 11, 2020

raftstore: Remove future_poller pool and batch Ticks (#8457) (#8634)

da63556

* cherry pick #8457 to release-4.0 Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ldeng-ustc pushed a commit to ldeng-ustc/tikv that referenced this pull request Sep 12, 2020

raftstore: Remove future_poller pool and batch Ticks (tikv#8457)

41cc00c

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace deleted the heartbeat branch November 23, 2021 08:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

raftstore: Remove future_poller pool and batch Ticks #8457

raftstore: Remove future_poller pool and batch Ticks #8457

Little-Wallace commented Aug 17, 2020

Little-Wallace commented Aug 17, 2020

gengliqi Aug 19, 2020

gengliqi Aug 20, 2020

Little-Wallace Aug 26, 2020

BusyJay Aug 20, 2020

Little-Wallace Aug 25, 2020

NingLin-P Aug 27, 2020

Little-Wallace Aug 28, 2020

Little-Wallace commented Aug 30, 2020

NingLin-P Aug 31, 2020

Little-Wallace Aug 31, 2020

codecov bot commented Aug 31, 2020 •

edited

NingLin-P left a comment

ti-srebot commented Sep 7, 2020

Little-Wallace commented Sep 7, 2020

Little-Wallace commented Sep 7, 2020

gengliqi Sep 9, 2020

gengliqi Sep 9, 2020

gengliqi left a comment

ti-srebot commented Sep 9, 2020

gengliqi commented Sep 9, 2020

ti-srebot commented Sep 9, 2020

ti-srebot commented Sep 9, 2020

	// update config
	self.poll_ctx.perf_context_statistics.start();
	if let Some(incoming) = self.cfg_tracker.any_new() {
	match Ord::cmp(
	&incoming.messages_per_tick,
	&self.poll_ctx.cfg.messages_per_tick,
	) {
	CmpOrdering::Greater => {
	self.store_msg_buf.reserve(incoming.messages_per_tick);
	self.peer_msg_buf.reserve(incoming.messages_per_tick);
	self.messages_per_tick = incoming.messages_per_tick;
	}
	CmpOrdering::Less => {
	self.store_msg_buf.shrink_to(incoming.messages_per_tick);
	self.peer_msg_buf.shrink_to(incoming.messages_per_tick);
	self.messages_per_tick = incoming.messages_per_tick;
	}
	_ => {}
	}
	self.poll_ctx.cfg = incoming.clone();
	}

raftstore: Remove future_poller pool and batch Ticks #8457

raftstore: Remove future_poller pool and batch Ticks #8457

Conversation

Little-Wallace commented Aug 17, 2020

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

Little-Wallace commented Aug 17, 2020

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

Little-Wallace commented Aug 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 31, 2020 • edited

Codecov Report

NingLin-P left a comment

Choose a reason for hiding this comment

ti-srebot commented Sep 7, 2020

Little-Wallace commented Sep 7, 2020

Little-Wallace commented Sep 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gengliqi left a comment

Choose a reason for hiding this comment

ti-srebot commented Sep 9, 2020

gengliqi commented Sep 9, 2020

ti-srebot commented Sep 9, 2020

ti-srebot commented Sep 9, 2020

codecov bot commented Aug 31, 2020 •

edited