keyspace, slice: improve code efficiency in membership ops (#6392)

ref #6231

Improve code efficiency in membership ops

Signed-off-by: Bin Shi <[email protected]>

pkg/keyspace/tso_keyspace_group.go

@@ -516,21 +516,30 @@ func (m *GroupManager) updateKeyspaceForGroupLocked(userKind endpoint.UserKind,
 	if kg.IsSplitting() {
 		return ErrKeyspaceGroupInSplit
 	}
+
+	changed := false
+
 	switch mutation {
 	case opAdd:
 		if !slice.Contains(kg.Keyspaces, keyspaceID) {
 			kg.Keyspaces = append(kg.Keyspaces, keyspaceID)
+			changed = true
 		}
 	case opDelete:
-		if slice.Contains(kg.Keyspaces, keyspaceID) {
-			kg.Keyspaces = slice.Remove(kg.Keyspaces, keyspaceID)
+		lenOfKeyspaces := len(kg.Keyspaces)
+		kg.Keyspaces = slice.Remove(kg.Keyspaces, keyspaceID)
+		if lenOfKeyspaces != len(kg.Keyspaces) {
+			changed = true
 		}
 	}
-	if err := m.saveKeyspaceGroups([]*endpoint.KeyspaceGroup{kg}, true); err != nil {
-		return err
-	}
 
-	m.groups[userKind].Put(kg)
+	if changed {
+		if err := m.saveKeyspaceGroups([]*endpoint.KeyspaceGroup{kg}, true); err != nil {
+			return err
+		}
+
+		m.groups[userKind].Put(kg)
+	}
 	return nil
 }
 
@@ -569,8 +578,9 @@ func (m *GroupManager) UpdateKeyspaceGroup(oldGroupID, newGroupID string, oldUse
 		updateNew = true
 	}
 
-	if slice.Contains(oldKG.Keyspaces, keyspaceID) {
-		oldKG.Keyspaces = slice.Remove(oldKG.Keyspaces, keyspaceID)
+	lenOfOldKeyspaces := len(oldKG.Keyspaces)
+	oldKG.Keyspaces = slice.Remove(oldKG.Keyspaces, keyspaceID)
+	if lenOfOldKeyspaces != len(oldKG.Keyspaces) {
 		updateOld = true
 	}
 

pkg/slice/slice.go

@@ -49,11 +49,12 @@ func Contains[T comparable](slice []T, value T) bool {
 
 // Remove removes the value from the slice.
 func Remove[T comparable](slice []T, value T) []T {
-	for i := 0; i < len(slice); i++ {
-		if slice[i] == value {
-			slice = append(slice[:i], slice[i+1:]...)
-			i--
+	i, j := 0, 0
+	for ; i < len(slice); i++ {
+		if slice[i] != value {
+			slice[j] = slice[i]
+			j++
 		}
 	}
-	return slice
+	return slice[:j]
 }

pkg/slice/slice_test.go

@@ -60,7 +60,7 @@ func TestSliceContains(t *testing.T) {
 	re.False(slice.Contains(is, int64(4)))
 }
 
-func TestSliceRemove(t *testing.T) {
+func TestSliceRemoveGenericTypes(t *testing.T) {
 	t.Parallel()
 	re := require.New(t)
 	ss := []string{"a", "b", "c"}
@@ -75,3 +75,26 @@ func TestSliceRemove(t *testing.T) {
 	is = slice.Remove(is, 1)
 	re.Equal([]int64{2, 3}, is)
 }
+
+func TestSliceRemove(t *testing.T) {
+	t.Parallel()
+	re := require.New(t)
+
+	is := []int64{}
+	is = slice.Remove(is, 1)
+	re.Equal([]int64{}, is)
+
+	is = []int64{1}
+	is = slice.Remove(is, 2)
+	re.Equal([]int64{1}, is)
+	is = slice.Remove(is, 1)
+	re.Equal([]int64{}, is)
+
+	is = []int64{1, 2, 3}
+	is = slice.Remove(is, 1)
+	re.Equal([]int64{2, 3}, is)
+
+	is = []int64{1, 1, 1}
+	is = slice.Remove(is, 1)
+	re.Equal([]int64{}, is)
+}

tests/integrations/mcs/keyspace/tso_keyspace_group_test.go

@@ -82,7 +82,7 @@ func (suite *keyspaceGroupTestSuite) TearDownTest() {
 func (suite *keyspaceGroupTestSuite) TestAllocNodesUpdate() {
 	// add three nodes.
 	nodes := make(map[string]bs.Server)
-	for i := 0; i < utils.KeyspaceGroupDefaultReplicaCount+2; i++ {
+	for i := 0; i < utils.KeyspaceGroupDefaultReplicaCount+1; i++ {
 		s, cleanup := mcs.StartSingleTSOTestServer(suite.ctx, suite.Require(), suite.backendEndpoints, tempurl.Alloc())
 		defer cleanup()
 		nodes[s.GetAddr()] = s
@@ -113,7 +113,7 @@ func (suite *keyspaceGroupTestSuite) TestAllocNodesUpdate() {
 		oldMembers[member.Address] = struct{}{}
 	}
 
-	// alloc node update to 2.
+	// alloc node update to 3.
 	params.Replica = utils.KeyspaceGroupDefaultReplicaCount + 1
 	got, code = suite.tryAllocNodesForKeyspaceGroup(id, params)
 	suite.Equal(http.StatusOK, code)