Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/6946

to look at the new patch set (#7).

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

consensus: consolidate Raft thread pools

This patch consolidates the two thread pools used in Raft consensus: the
observer and "Raft" (for lack of a better name) pools. The former runs tasks
for infrequent events such as term and config changes while the latter is
used for periodic events such as processing heartbeat responses.

Both per-replica pools are consolidated into a single per-server pool.
Sequence tokens are used to ensure that tasks belonging to a single replica
are executed serially. One token is shared by RaftConsensus (primary) and
PeerManager (secondary); this mirrors the existing sharing behavior and is
safe because token operations are thread-safe. A second token is dedicated
for the observer submissions, so that its (potentially) long-running tasks
don't starve any periodic Raft events.

The non-default max_threads may come as a surprise, but David showed me how
tasks submitted to either pool may sometimes lead to an fsync (mostly via
cmeta flushes). As such, it isn't safe to use the default num_cpus upper
bound, as that may cause such IO-based tasks to block other CPU-only tasks
(or worse, periodic tasks like heartbeat response processing).

What per-replica threads are not addressed by this patch?
- Failure detection thread: a threadpool isn't the right model
  for these. Something like a hash wheel timer would be ideal.
- Prepare thread pool (max_threads=1): these could be consolidated too, but
  their metrics are per-replica, and sequence tokens don't (yet) support
  that. Meaning, if they were consolidated now, the metrics would also
  consolidate and would be less useful as a result.

Change-Id: I8958c607d11ac2e94908670c985059bef0ff5f54
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_peers.cc
M src/kudu/consensus/consensus_peers.h
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/peer_manager.cc
M src/kudu/consensus/peer_manager.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
19 files changed, 195 insertions(+), 119 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/6946/7
-- 
To view, visit http://gerrit.cloudera.org:8080/6946
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8958c607d11ac2e94908670c985059bef0ff5f54
Gerrit-PatchSet: 7
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: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>

Reply via email to