David Ribeiro Alves has posted comments on this change. Change subject: consensus: consolidate Raft thread pools ......................................................................
Patch Set 13: (3 comments) http://gerrit.cloudera.org:8080/#/c/6946/13/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: PS13, Line 191: raft_pool->NewToken(ThreadPool::ExecutionMode::SERIAL) it's misleading to name both tokens (the queue and the raft proper one) as "raft_pool_tokens" since it leads to thinking they are the same http://gerrit.cloudera.org:8080/#/c/6946/13/src/kudu/consensus/raft_consensus.h File src/kudu/consensus/raft_consensus.h: PS13, Line 121: ConsensusOptions options, : std::unique_ptr<ConsensusMetadata> cmeta, : const RaftPeerPB& local_peer_pb, : const scoped_refptr<MetricEntity>& metric_entity, : scoped_refptr<TimeManager> time_manager, : ReplicaTransactionFactory* txn_factory, : const std::shared_ptr<rpc::Messenger>& messenger, : const scoped_refptr<log::Log>& log, : const std::shared_ptr<MemTracker>& parent_mem_tracker, : const Callback<void(const std::string& reason)>& mark_dirty_clbk, : ThreadPool* raft_pool); heads up, this is going to conflict with mike's in flight patches http://gerrit.cloudera.org:8080/#/c/6946/13/src/kudu/kserver/kserver.cc File src/kudu/kserver/kserver.cc: PS13, Line 106: if (raft_pool_) { : raft_pool_->Shutdown(); : } is ordering important here? I would think that shutting down the apply pool first would be problematic since there might be stuff in flight to still be applied as we close the other pools first. am I missing something? -- 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: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
