From 58f78df1deea73793cfd2c274b4624836d480a0a Mon Sep 17 00:00:00 2001
From: Piotr Tabor <ptab@google.com>
Date: Wed, 30 Dec 2020 16:28:56 +0100
Subject: [PATCH] Raft: Expand raft documentation, in particular point on
 godocs.

---
 api/etcdserverpb/rpc.proto | 4 +++-
 raft/README.md             | 4 ++++
 raft/raftpb/raft.proto     | 2 ++
 server/wal/wal.go          | 8 ++++++++
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/api/etcdserverpb/rpc.proto b/api/etcdserverpb/rpc.proto
index e63d4057b..14391378a 100644
--- a/api/etcdserverpb/rpc.proto
+++ b/api/etcdserverpb/rpc.proto
@@ -238,7 +238,9 @@ service Maintenance {
     };
   }
 
-  // Downgrade requests downgrade, cancel downgrade on the cluster version.
+  // Downgrade requests downgrades, verifies feasibility or cancels downgrade
+  // on the cluster version.
+  // Supported since etcd 3.5.
   rpc Downgrade(DowngradeRequest) returns (DowngradeResponse) {
     option (google.api.http) = {
       post: "/v3/maintenance/downgrade"
diff --git a/raft/README.md b/raft/README.md
index 5f67517fc..c25a176ea 100644
--- a/raft/README.md
+++ b/raft/README.md
@@ -195,3 +195,7 @@ This implementation is up to date with the final Raft thesis (https://github.com
 To ensure there is no attempt to commit two membership changes at once by matching log positions (which would be unsafe since they should have different quorum requirements), any proposed membership change is simply disallowed while any uncommitted change appears in the leader's log.
 
 This approach introduces a problem when removing a member from a two-member cluster: If one of the members dies before the other one receives the commit of the confchange entry, then the member cannot be removed any more since the cluster cannot make progress. For this reason it is highly recommended to use three or more nodes in every cluster.
+
+## Go docs
+
+More detailed development documentation can be found in go docs: https://pkg.go.dev/go.etcd.io/etcd/raft/v3.
\ No newline at end of file
diff --git a/raft/raftpb/raft.proto b/raft/raftpb/raft.proto
index 23d62ec2f..ed4e45151 100644
--- a/raft/raftpb/raft.proto
+++ b/raft/raftpb/raft.proto
@@ -33,6 +33,8 @@ message Snapshot {
 	optional SnapshotMetadata metadata = 2 [(gogoproto.nullable) = false];
 }
 
+// For description of different message types, see:
+// https://pkg.go.dev/go.etcd.io/etcd/raft/v3#hdr-MessageType
 enum MessageType {
 	MsgHup             = 0;
 	MsgBeat            = 1;
diff --git a/server/wal/wal.go b/server/wal/wal.go
index bf820a61f..37f6dd136 100644
--- a/server/wal/wal.go
+++ b/server/wal/wal.go
@@ -419,6 +419,13 @@ func openWALFiles(lg *zap.Logger, dirpath string, names []string, nameIndex int,
 // TODO: detect not-last-snap error.
 // TODO: maybe loose the checking of match.
 // After ReadAll, the WAL will be ready for appending new records.
+//
+// ReadAll suppresses WAL entries that got overridden (i.e. a newer entry with the same index
+// exists in the log). Such a situation can happen in cases described in figure 7. of the
+// RAFT paper (http://web.stanford.edu/~ouster/cgi-bin/papers/raft-atc14.pdf).
+//
+// ReadAll may return uncommitted yet entries, that are subject to be overriden.
+// Do not apply entries that have index > state.commit, as they are subject to change.
 func (w *WAL) ReadAll() (metadata []byte, state raftpb.HardState, ents []raftpb.Entry, err error) {
 	w.mu.Lock()
 	defer w.mu.Unlock()
@@ -443,6 +450,7 @@ func (w *WAL) ReadAll() (metadata []byte, state raftpb.HardState, ents []raftpb.
 					// return error before append call causes runtime panic
 					return nil, state, nil, ErrSliceOutOfRange
 				}
+				// The line below is potentially overriding some 'uncommitted' entries.
 				ents = append(ents[:up], e)
 			}
 			w.enti = e.Index