From 08712c3ef170daa0eff1da818b37b55344f49d98 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 17 Jan 2022 13:33:56 -0500 Subject: [PATCH 1/2] Update godoc for Staging and Promote --- configuration.go | 11 +++++------ raft.go | 4 +--- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/configuration.go b/configuration.go index 5c6636058..de2f279d7 100644 --- a/configuration.go +++ b/configuration.go @@ -13,10 +13,9 @@ const ( // Nonvoter is a server that receives log entries but is not considered for // elections or commitment purposes. Nonvoter - // Staging is a server that acts like a nonvoter with one exception: once a - // staging server receives enough log entries to be sufficiently caught up to - // the leader's log, the leader will invoke a membership change to change - // the Staging server to a Voter. + // Staging is a server that acts like a Nonvoter. A configuration change + // with a ConfigurationChangeCommand of Promote can change a Staging server + // into a Voter. Staging ) @@ -95,8 +94,8 @@ const ( DemoteVoter // RemoveServer removes a server entirely from the cluster membership. RemoveServer - // Promote is created automatically by a leader; it turns a Staging server - // into a Voter. + // Promote changes a server from Staging to Voter. The command will be a + // no-op if the server is not Staging. Promote ) diff --git a/raft.go b/raft.go index bd6b2b792..95d487c40 100644 --- a/raft.go +++ b/raft.go @@ -1370,9 +1370,7 @@ func (r *Raft) appendEntries(rpc RPC, a *AppendEntriesRequest) { return } if entry.Term != storeEntry.Term { - r.logger.Warn("clearing log suffix", - "from", entry.Index, - "to", lastLogIdx) + r.logger.Warn("clearing log suffix", "from", entry.Index, "to", lastLogIdx) if err := r.logs.DeleteRange(entry.Index, lastLogIdx); err != nil { r.logger.Error("failed to clear log suffix", "error", err) return From 0a8bb8cf6f4eb9a2436a1d3fe179ff65df82efd0 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 17 Jan 2022 13:46:32 -0500 Subject: [PATCH 2/2] Deprecate AddStaging and Promote Replace it with AddVoter, which is how this has worked for the last 5 years. This change should be backwards compatible. The old commands still work, but are now an alias for AddVoter. --- api.go | 4 ++-- configuration.go | 24 +++++++++++------------- configuration_test.go | 16 +++++++++++++--- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/api.go b/api.go index 9152cf620..440de7b96 100644 --- a/api.go +++ b/api.go @@ -825,7 +825,7 @@ func (r *Raft) AddPeer(peer ServerAddress) Future { } return r.requestConfigChange(configurationChangeRequest{ - command: AddStaging, + command: AddVoter, serverID: ServerID(peer), serverAddress: peer, prevIndex: 0, @@ -862,7 +862,7 @@ func (r *Raft) AddVoter(id ServerID, address ServerAddress, prevIndex uint64, ti } return r.requestConfigChange(configurationChangeRequest{ - command: AddStaging, + command: AddVoter, serverID: id, serverAddress: address, prevIndex: prevIndex, diff --git a/configuration.go b/configuration.go index de2f279d7..b4c78d290 100644 --- a/configuration.go +++ b/configuration.go @@ -16,6 +16,7 @@ const ( // Staging is a server that acts like a Nonvoter. A configuration change // with a ConfigurationChangeCommand of Promote can change a Staging server // into a Voter. + // Deprecated: use Nonvoter instead. Staging ) @@ -86,8 +87,8 @@ func (c *Configuration) Clone() (copy Configuration) { type ConfigurationChangeCommand uint8 const ( - // AddStaging makes a server Staging unless its Voter. - AddStaging ConfigurationChangeCommand = iota + // AddVoter adds a server with Suffrage of Voter. + AddVoter ConfigurationChangeCommand = iota // AddNonvoter makes a server Nonvoter unless its Staging or Voter. AddNonvoter // DemoteVoter makes a server Nonvoter unless its absent. @@ -96,13 +97,17 @@ const ( RemoveServer // Promote changes a server from Staging to Voter. The command will be a // no-op if the server is not Staging. + // Deprecated: use AddVoter instead. Promote + // AddStaging makes a server a Voter. + // Deprecated: AddStaging was actually AddVoter. Use AddVoter instead. + AddStaging = 0 // explicit 0 to preserve the old value. ) func (c ConfigurationChangeCommand) String() string { switch c { - case AddStaging: - return "AddStaging" + case AddVoter: + return "AddVoter" case AddNonvoter: return "AddNonvoter" case DemoteVoter: @@ -121,7 +126,7 @@ func (c ConfigurationChangeCommand) String() string { type configurationChangeRequest struct { command ConfigurationChangeCommand serverID ServerID - serverAddress ServerAddress // only present for AddStaging, AddNonvoter + serverAddress ServerAddress // only present for AddVoter, AddNonvoter // prevIndex, if nonzero, is the index of the only configuration upon which // this change may be applied; if another configuration entry has been // added in the meantime, this request will fail. @@ -213,15 +218,8 @@ func nextConfiguration(current Configuration, currentIndex uint64, change config configuration := current.Clone() switch change.command { - case AddStaging: - // TODO: barf on new address? + case AddVoter: newServer := Server{ - // TODO: This should add the server as Staging, to be automatically - // promoted to Voter later. However, the promotion to Voter is not yet - // implemented, and doing so is not trivial with the way the leader loop - // coordinates with the replication goroutines today. So, for now, the - // server will have a vote right away, and the Promote case below is - // unused. Suffrage: Voter, ID: change.serverID, Address: change.serverAddress, diff --git a/configuration_test.go b/configuration_test.go index edbe545b1..b9883612a 100644 --- a/configuration_test.go +++ b/configuration_test.go @@ -179,6 +179,16 @@ var nextConfigurationTests = []struct { // AddStaging: was Nonvoter. {oneOfEach, AddStaging, 3, "{[{Voter id1 addr1x} {Staging id2 addr2x} {Voter id3 addr3}]}"}, + // AddVoter: was missing. + {Configuration{}, AddVoter, 1, "{[{Voter id1 addr1}]}"}, + {singleServer, AddVoter, 2, "{[{Voter id1 addr1x} {Voter id2 addr2}]}"}, + // AddVoter: was Voter. + {singleServer, AddVoter, 1, "{[{Voter id1 addr1}]}"}, + // AddVoter: was Staging. + {oneOfEach, AddVoter, 2, "{[{Voter id1 addr1x} {Voter id2 addr2} {Nonvoter id3 addr3x}]}"}, + // AddVoter: was Nonvoter. + {oneOfEach, AddVoter, 3, "{[{Voter id1 addr1x} {Staging id2 addr2x} {Voter id3 addr3}]}"}, + // AddNonvoter: was missing. {singleServer, AddNonvoter, 2, "{[{Voter id1 addr1x} {Nonvoter id2 addr2}]}"}, // AddNonvoter: was Voter. @@ -238,7 +248,7 @@ func TestConfiguration_nextConfiguration_table(t *testing.T) { func TestConfiguration_nextConfiguration_prevIndex(t *testing.T) { // Stale prevIndex. req := configurationChangeRequest{ - command: AddStaging, + command: AddVoter, serverID: ServerID("id1"), serverAddress: ServerAddress("addr1"), prevIndex: 1, @@ -250,7 +260,7 @@ func TestConfiguration_nextConfiguration_prevIndex(t *testing.T) { // Current prevIndex. req = configurationChangeRequest{ - command: AddStaging, + command: AddVoter, serverID: ServerID("id2"), serverAddress: ServerAddress("addr2"), prevIndex: 2, @@ -262,7 +272,7 @@ func TestConfiguration_nextConfiguration_prevIndex(t *testing.T) { // Zero prevIndex. req = configurationChangeRequest{ - command: AddStaging, + command: AddVoter, serverID: ServerID("id3"), serverAddress: ServerAddress("addr3"), prevIndex: 0,