Hello David Ribeiro Alves, Mike Percy,
I'd like you to do a code review. Please visit
to review the following change.
Change subject: consensus: move more logic from ReplicaState to RaftConsensus
consensus: move more logic from ReplicaState to RaftConsensus
ReplicaState currently has two main responsibilities:
* manage the set of pending transactions, including committing
and aborting them. The main state here is the pending_txns_
* act as a wrapper to ConsensusMeta (stored as cmeta_).
These two items are relatively distinct. I drew a graph of the
dependencies between methods and members of ReplicaState and it
became clear that most methods either pertained to one or the
other of these responsibilities, but not both. So, this patch seeks to
try to clean up the methods which were straddling the two
responsibilities as follows:
1. AbortOpsAfterUnlocked, AddPendingOperation, AdvanceCommittedIndexUnlocked
These three are mainly focused around managing pending transactions,
but were also taking care of setting the pending and committed Raft
configuration in the case of configuration change operations.
The patch moves the handling of configuration changes into
RaftConsensus itself by using the existing "replicated callback" to
perform the necessary work when the operation finishes, and by
checking for config changes before adding pending operations.
This one is used to check whether the last committed operation is in
the current term. Recently the PeerMessageQueue became responsible for
tracking the committed OpId, and already has the necessary state to
provide the same functionality.
After the refactor, the class clearly shows two separate groups of
methods and members. This will make it easier to maintain/understand
the code as well as open more opportunities for finer-grained locking by
having smaller classes with better-defined responsibilities.
I looped raft_consensus-itest 100 times with this patch and passed
NOTE: for the sake of the dependency graphs, I removed the state_
variable, ToString methods, and locking methods and left only the
7 files changed, 121 insertions(+), 98 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/4709/1
To view, visit http://gerrit.cloudera.org:8080/4709
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>