Skip to content

Commit

Permalink
Merge pull request #17056 from serathius/revert-switch-v3
Browse files Browse the repository at this point in the history
Revert "Switch to validating v3 when v2 and v3 are synchronized"
  • Loading branch information
serathius authored Dec 3, 2023
2 parents 21704b8 + bc697bc commit 93ab2ef
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 19 deletions.
13 changes: 3 additions & 10 deletions server/etcdserver/api/membership/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,16 +303,9 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) {

// ValidateConfigurationChange takes a proposed ConfChange and
// ensures that it is still valid.
func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange, shouldApplyV3 ShouldApplyV3) error {
var membersMap map[types.ID]*Member
var removedMap map[types.ID]bool

if shouldApplyV3 {
membersMap, removedMap = c.be.MustReadMembersFromBackend()
} else {
membersMap, removedMap = membersFromStore(c.lg, c.v2store)
}

func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error {
// TODO: this must be switched to backend as well.
membersMap, removedMap := membersFromStore(c.lg, c.v2store)
id := types.ID(cc.NodeID)
if removedMap[id] {
return ErrIDRemoved
Expand Down
9 changes: 1 addition & 8 deletions server/etcdserver/api/membership/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,14 +276,7 @@ func TestClusterValidateAndAssignIDs(t *testing.T) {
}
}

func TestClusterValidateConfigurationChangeV3(t *testing.T) {
testClusterValidateConfigurationChange(t, true)
}
func TestClusterValidateConfigurationChangeV2(t *testing.T) {
testClusterValidateConfigurationChange(t, false)
}

func testClusterValidateConfigurationChange(t *testing.T, shouldApplyV3 ShouldApplyV3) {
cl := NewCluster(zaptest.NewLogger(t), WithMaxLearners(1))
be := newMembershipBackend()
cl.SetBackend(be)
Expand Down Expand Up @@ -464,7 +457,7 @@ func testClusterValidateConfigurationChange(t *testing.T, shouldApplyV3 ShouldAp
},
}
for i, tt := range tests {
err := cl.ValidateConfigurationChange(tt.cc, shouldApplyV3)
err := cl.ValidateConfigurationChange(tt.cc)
if err != tt.werr {
t.Errorf("#%d: validateConfigurationChange error = %v, want %v", i, err, tt.werr)
}
Expand Down
2 changes: 1 addition & 1 deletion server/etcdserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1967,7 +1967,7 @@ func removeNeedlessRangeReqs(txn *pb.TxnRequest) {
// applyConfChange applies a ConfChange to the server. It is only
// invoked with a ConfChange that has already passed through Raft
func (s *EtcdServer) applyConfChange(cc raftpb.ConfChange, confState *raftpb.ConfState, shouldApplyV3 membership.ShouldApplyV3) (bool, error) {
if err := s.cluster.ValidateConfigurationChange(cc, shouldApplyV3); err != nil {
if err := s.cluster.ValidateConfigurationChange(cc); err != nil {
cc.NodeID = raft.None
s.r.ApplyConfChange(cc)

Expand Down

0 comments on commit 93ab2ef

Please sign in to comment.