Merge pull request #485 from hashicorp/dnephin/godoc-1

Miscellaneous godoc and comment improvements

Author: dnephin

api.go

@@ -15,15 +15,16 @@ import (
 )
 
 const (
-	// This is the current suggested max size of the data in a raft log entry.
-	// This is based on current architecture, default timing, etc. Clients can
+	// SuggestedMaxDataSize of the data in a raft log entry, in bytes.
+	//
+	// The value is based on current architecture, default timing, etc. Clients can
 	// ignore this value if they want as there is no actual hard checking
 	// within the library. As the library is enhanced this value may change
 	// over time to reflect current suggested maximums.
 	//
-	// Increasing beyond this risks RPC IO taking too long and preventing
-	// timely heartbeat signals which are sent in serial in current transports,
-	// potentially causing leadership instability.
+	// Applying log entries with data greater than this size risks RPC IO taking
+	// too long and preventing timely heartbeat signals.  These signals are sent in serial
+	// in current transports, potentially causing leadership instability.
 	SuggestedMaxDataSize = 512 * 1024
 )
 
@@ -146,8 +147,8 @@ type Raft struct {
 	// the log/snapshot.
 	configurations configurations
 
-	// Holds a copy of the latest configuration which can be read
-	// independently from main loop.
+	// Holds a copy of the latest configuration which can be read independently
+	// of the main loop.
 	latestConfiguration atomic.Value
 
 	// RPC chan comes from the transport layer
@@ -762,11 +763,11 @@ func (r *Raft) ApplyLog(log Log, timeout time.Duration) ApplyFuture {
 	}
 }
 
-// Barrier is used to issue a command that blocks until all preceeding
+// Barrier is used to issue a command that blocks until all preceding
 // operations have been applied to the FSM. It can be used to ensure the
 // FSM reflects all queued writes. An optional timeout can be provided to
 // limit the amount of time we wait for the command to be started. This
-// must be run on the leader or it will fail.
+// must be run on the leader, or it will fail.
 func (r *Raft) Barrier(timeout time.Duration) Future {
 	metrics.IncrCounter([]string{"raft", "barrier"}, 1)
 	var timer <-chan time.Time
@@ -775,11 +776,7 @@ func (r *Raft) Barrier(timeout time.Duration) Future {
 	}
 
 	// Create a log future, no index or term yet
-	logFuture := &logFuture{
-		log: Log{
-			Type: LogBarrier,
-		},
-	}
+	logFuture := &logFuture{log: Log{Type: LogBarrier}}
 	logFuture.init()
 
 	select {
@@ -792,9 +789,9 @@ func (r *Raft) Barrier(timeout time.Duration) Future {
 	}
 }
 
-// VerifyLeader is used to ensure the current node is still
-// the leader. This can be done to prevent stale reads when a
-// new leader has potentially been elected.
+// VerifyLeader is used to ensure this peer is still the leader. It may be used
+// to prevent returning stale data from the FSM after the peer has lost
+// leadership.
 func (r *Raft) VerifyLeader() Future {
 	metrics.IncrCounter([]string{"raft", "verify_leader"}, 1)
 	verifyFuture := &verifyFuture{}
@@ -817,8 +814,9 @@ func (r *Raft) GetConfiguration() ConfigurationFuture {
 	return configReq
 }
 
-// AddPeer (deprecated) is used to add a new peer into the cluster. This must be
-// run on the leader or it will fail. Use AddVoter/AddNonvoter instead.
+// AddPeer to the cluster configuration. Must be run on the leader, or it will fail.
+//
+// Deprecated: Use AddVoter/AddNonvoter instead.
 func (r *Raft) AddPeer(peer ServerAddress) Future {
 	if r.protocolVersion > 2 {
 		return errorFuture{ErrUnsupportedProtocol}
@@ -832,10 +830,11 @@ func (r *Raft) AddPeer(peer ServerAddress) Future {
 	}, 0)
 }
 
-// RemovePeer (deprecated) is used to remove a peer from the cluster. If the
-// current leader is being removed, it will cause a new election
-// to occur. This must be run on the leader or it will fail.
-// Use RemoveServer instead.
+// RemovePeer from the cluster configuration. If the current leader is being
+// removed, it will cause a new election to occur. Must be run on the leader,
+// or it will fail.
+
+// Deprecated: Use RemoveServer instead.
 func (r *Raft) RemovePeer(peer ServerAddress) Future {
 	if r.protocolVersion > 2 {
 		return errorFuture{ErrUnsupportedProtocol}
@@ -955,7 +954,7 @@ func (r *Raft) Snapshot() SnapshotFuture {
 // Restore is used to manually force Raft to consume an external snapshot, such
 // as if restoring from a backup. We will use the current Raft configuration,
 // not the one from the snapshot, so that we can restore into a new cluster. We
-// will also use the higher of the index of the snapshot, or the current index,
+// will also use the max of the index of the snapshot, or the current index,
 // and then add 1 to that, so we force a new state with a hole in the Raft log,
 // so that the snapshot will be sent to followers and used for any new joiners.
 // This can only be run on the leader, and blocks until the restore is complete
@@ -1011,7 +1010,7 @@ func (r *Raft) Restore(meta *SnapshotMeta, reader io.Reader, timeout time.Durati
 	}
 }
 
-// State is used to return the current raft state.
+// State returns the state of this raft peer.
 func (r *Raft) State() RaftState {
 	return r.getState()
 }
@@ -1151,7 +1150,7 @@ func (r *Raft) AppliedIndex() uint64 {
 // This can only be called from the leader, or it will fail. The leader will
 // stop accepting client requests, make sure the target server is up to date
 // and starts the transfer with a TimeoutNow message. This message has the same
-// effect as if the election timeout on the on the target server fires. Since
+// effect as if the election timeout on the target server fires. Since
 // it is unlikely that another server is starting an election, it is very
 // likely that the target server is able to win the election.  Note that raft
 // protocol version 3 is not sufficient to use LeadershipTransfer. A recent

config.go

@@ -132,17 +132,18 @@ type Config struct {
 	// can _understand_.
 	ProtocolVersion ProtocolVersion
 
-	// HeartbeatTimeout specifies the time in follower state without
-	// a leader before we attempt an election.
+	// HeartbeatTimeout specifies the time in follower state without contact
+	// from a leader before we attempt an election.
 	HeartbeatTimeout time.Duration
 
-	// ElectionTimeout specifies the time in candidate state without
-	// a leader before we attempt an election.
+	// ElectionTimeout specifies the time in candidate state without contact
+	// from a leader before we attempt an election.
 	ElectionTimeout time.Duration
 
-	// CommitTimeout controls the time without an Apply() operation
-	// before we heartbeat to ensure a timely commit. Due to random
-	// staggering, may be delayed as much as 2x this value.
+	// CommitTimeout specifies the time without an Apply operation before the
+	// leader sends an AppendEntry RPC to followers, to ensure a timely commit of
+	// log entries.
+	// Due to random staggering, may be delayed as much as 2x this value.
 	CommitTimeout time.Duration
 
 	// MaxAppendEntries controls the maximum number of append entries

fsm.go

@@ -8,27 +8,34 @@ import (
 	"github.com/armon/go-metrics"
 )
 
-// FSM provides an interface that can be implemented by
-// clients to make use of the replicated log.
+// FSM is implemented by clients to make use of the replicated log.
 type FSM interface {
-	// Apply log is invoked once a log entry is committed.
-	// It returns a value which will be made available in the
-	// ApplyFuture returned by Raft.Apply method if that
-	// method was called on the same Raft node as the FSM.
+	// Apply is called once a log entry is committed by a majority of the cluster.
+	//
+	// Apply should apply the log to the FSM. Apply must be deterministic and
+	// produce the same result on all peers in the cluster.
+	//
+	// The returned value is returned to the client as the ApplyFuture.Response.
 	Apply(*Log) interface{}
 
-	// Snapshot is used to support log compaction. This call should
-	// return an FSMSnapshot which can be used to save a point-in-time
-	// snapshot of the FSM. Apply and Snapshot are not called in multiple
-	// threads, but Apply will be called concurrently with Persist. This means
-	// the FSM should be implemented in a fashion that allows for concurrent
-	// updates while a snapshot is happening.
+	// Snapshot returns an FSMSnapshot used to: support log compaction, to
+	// restore the FSM to a previous state, or to bring out-of-date followers up
+	// to a recent log index.
+	//
+	// The Snapshot implementation should return quickly, because Apply can not
+	// be called while Snapshot is running. Generally this means Snapshot should
+	// only capture a pointer to the state, and any expensive IO should happen
+	// as part of FSMSnapshot.Persist.
+	//
+	// Apply and Snapshot are always called from the same thread, but Apply will
+	// be called concurrently with FSMSnapshot.Persist. This means the FSM should
+	// be implemented to allow for concurrent updates while a snapshot is happening.
 	Snapshot() (FSMSnapshot, error)
 
 	// Restore is used to restore an FSM from a snapshot. It is not called
 	// concurrently with any other command. The FSM must discard all previous
-	// state.
-	Restore(io.ReadCloser) error
+	// state before restoring the snapshot.
+	Restore(snapshot io.ReadCloser) error
 }
 
 // BatchingFSM extends the FSM interface to add an ApplyBatch function. This can
@@ -72,7 +79,7 @@ func (r *Raft) runFSM() {
 	batchingFSM, batchingEnabled := r.fsm.(BatchingFSM)
 	configStore, configStoreEnabled := r.fsm.(ConfigurationStore)
 
-	commitSingle := func(req *commitTuple) {
+	applySingle := func(req *commitTuple) {
 		// Apply the log if a command or config change
 		var resp interface{}
 		// Make sure we send a response
@@ -107,10 +114,10 @@ func (r *Raft) runFSM() {
 		lastTerm = req.log.Term
 	}
 
-	commitBatch := func(reqs []*commitTuple) {
+	applyBatch := func(reqs []*commitTuple) {
 		if !batchingEnabled {
 			for _, ct := range reqs {
-				commitSingle(ct)
+				applySingle(ct)
 			}
 			return
 		}
@@ -213,7 +220,7 @@ func (r *Raft) runFSM() {
 		case ptr := <-r.fsmMutateCh:
 			switch req := ptr.(type) {
 			case []*commitTuple:
-				commitBatch(req)
+				applyBatch(req)
 
 			case *restoreFuture:
 				restore(req)

log.go

@@ -29,7 +29,7 @@ const (
 
 	// LogBarrier is used to ensure all preceding operations have been
 	// applied to the FSM. It is similar to LogNoop, but instead of returning
-	// once committed, it only returns once the FSM manager acks it. Otherwise
+	// once committed, it only returns once the FSM manager acks it. Otherwise,
 	// it is possible there are operations committed but not yet applied to
 	// the FSM.
 	LogBarrier

raft.go

@@ -123,7 +123,7 @@ func (r *Raft) requestConfigChange(req configurationChangeRequest, timeout time.
 	}
 }
 
-// run is a long running goroutine that runs the Raft FSM.
+// run the main thread that handles leadership and RPC requests.
 func (r *Raft) run() {
 	for {
 		// Check if we are doing a shutdown
@@ -135,7 +135,6 @@ func (r *Raft) run() {
 		default:
 		}
 
-		// Enter into a sub-FSM
 		switch r.getState() {
 		case Follower:
 			r.runFollower()
@@ -147,7 +146,7 @@ func (r *Raft) run() {
 	}
 }
 
-// runFollower runs the FSM for a follower.
+// runFollower runs the main loop while in the follower state.
 func (r *Raft) runFollower() {
 	didWarn := false
 	r.logger.Info("entering follower state", "follower", r, "leader", r.Leader())
@@ -236,8 +235,7 @@ func (r *Raft) runFollower() {
 func (r *Raft) liveBootstrap(configuration Configuration) error {
 	// Use the pre-init API to make the static updates.
 	cfg := r.config()
-	err := BootstrapCluster(&cfg, r.logs, r.stable, r.snapshots,
-		r.trans, configuration)
+	err := BootstrapCluster(&cfg, r.logs, r.stable, r.snapshots, r.trans, configuration)
 	if err != nil {
 		return err
 	}
@@ -252,7 +250,7 @@ func (r *Raft) liveBootstrap(configuration Configuration) error {
 	return r.processConfigurationLogEntry(&entry)
 }
 
-// runCandidate runs the FSM for a candidate.
+// runCandidate runs the main loop while in the candidate state.
 func (r *Raft) runCandidate() {
 	r.logger.Info("entering candidate state", "node", r, "term", r.getCurrentTerm()+1)
 	metrics.IncrCounter([]string{"raft", "state", "candidate"}, 1)
@@ -365,7 +363,7 @@ func (r *Raft) setupLeaderState() {
 	r.leaderState.stepDown = make(chan struct{}, 1)
 }
 
-// runLeader runs the FSM for a leader. Do the setup here and drop into
+// runLeader runs the main loop while in leader state. Do the setup here and drop into
 // the leaderLoop for the hot loop.
 func (r *Raft) runLeader() {
 	r.logger.Info("entering leader state", "leader", r)
@@ -464,11 +462,7 @@ func (r *Raft) runLeader() {
 	// an unbounded number of uncommitted configurations in the log. We now
 	// maintain that there exists at most one uncommitted configuration entry in
 	// any log, so we have to do proper no-ops here.
-	noop := &logFuture{
-		log: Log{
-			Type: LogNoop,
-		},
-	}
+	noop := &logFuture{log: Log{Type: LogNoop}}
 	r.dispatchLogs([]*logFuture{noop})
 
 	// Sit in the leader loop until we step down
@@ -659,7 +653,7 @@ func (r *Raft) leaderLoop() {
 			commitIndex := r.leaderState.commitment.getCommitIndex()
 			r.setCommitIndex(commitIndex)
 
-			// New configration has been committed, set it as the committed
+			// New configuration has been committed, set it as the committed
 			// value.
 			if r.configurations.latestIndex > oldCommitIndex &&
 				r.configurations.latestIndex <= commitIndex {
@@ -1449,7 +1443,7 @@ func (r *Raft) processConfigurationLogEntry(entry *Log) error {
 	return nil
 }
 
-// requestVote is invoked when we get an request vote RPC call.
+// requestVote is invoked when we get a request vote RPC call.
 func (r *Raft) requestVote(rpc RPC, req *RequestVoteRequest) {
 	defer metrics.MeasureSince([]string{"raft", "rpc", "requestVote"}, time.Now())
 	r.observe(*req)

util.go

@@ -32,7 +32,7 @@ func randomTimeout(minVal time.Duration) <-chan time.Time {
 	if minVal == 0 {
 		return nil
 	}
-	extra := (time.Duration(rand.Int63()) % minVal)
+	extra := time.Duration(rand.Int63()) % minVal
 	return time.After(minVal + extra)
 }