Dan Burkert has posted comments on this change.

Change subject: consensus: consolidate Raft thread pools
......................................................................


Patch Set 2:

(3 comments)

Nice.  Would be great to load this on to the flash cluster and see what the 
resulting difference in threads is for an otherwise idle cluster.  Each tserver 
on that cluster has ~5k tablets, so the savings could be substantial.

http://gerrit.cloudera.org:8080/#/c/6946/2/src/kudu/consensus/consensus_peers.h
File src/kudu/consensus/consensus_peers.h:

PS2, Line 130: )
extra parenthesis?


http://gerrit.cloudera.org:8080/#/c/6946/2/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

Line 191:   shared_ptr<SequenceToken> token(new 
SequenceToken(raft_pool->AllocateTokenSlot()));
I think this should be

shared_ptr<SequenceToken> token = make_shared(raft_pool->AllocateTokenSlot());


http://gerrit.cloudera.org:8080/#/c/6946/2/src/kudu/consensus/raft_consensus_quorum-test.cc
File src/kudu/consensus/raft_consensus_quorum-test.cc:

Line 168:           new SequenceToken(raft_pool_->AllocateTokenSlot()));
likewise


-- 
To view, visit http://gerrit.cloudera.org:8080/6946
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8958c607d11ac2e94908670c985059bef0ff5f54
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to