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