From d8a35411d010367cf83de90fcea002ab38a1bd98 Mon Sep 17 00:00:00 2001 From: JmPotato Date: Wed, 26 Apr 2023 16:17:51 +0800 Subject: [PATCH] keyspace, tso: check the replica count before the split (#6382) ref tikv/pd#6233 Check the replica count before the split. Signed-off-by: JmPotato Co-authored-by: lhy1024 Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> --- pkg/keyspace/tso_keyspace_group.go | 10 ++++++ pkg/keyspace/tso_keyspace_group_test.go | 5 +++ pkg/keyspace/util.go | 2 ++ pkg/tso/keyspace_group_manager.go | 36 ++++++++++++++++--- pkg/tso/keyspace_group_manager_test.go | 4 +-- .../apiv2/handlers/tso_keyspace_group_test.go | 1 + 6 files changed, 51 insertions(+), 7 deletions(-) diff --git a/pkg/keyspace/tso_keyspace_group.go b/pkg/keyspace/tso_keyspace_group.go index d1bfbbf59b6..b9e8eb311ff 100644 --- a/pkg/keyspace/tso_keyspace_group.go +++ b/pkg/keyspace/tso_keyspace_group.go @@ -552,6 +552,10 @@ func (m *GroupManager) SplitKeyspaceGroupByID(splitSourceID, splitTargetID uint3 if splitSourceKg.IsSplitting() { return ErrKeyspaceGroupInSplit } + // Check if the source keyspace group has enough replicas. + if len(splitSourceKg.Members) < utils.KeyspaceGroupDefaultReplicaCount { + return ErrKeyspaceGroupNotEnoughReplicas + } // Check if the new keyspace group already exists. splitTargetKg, err = m.store.LoadKeyspaceGroup(txn, splitTargetID) if err != nil { @@ -687,6 +691,9 @@ func (m *GroupManager) AllocNodesForKeyspaceGroup(id uint32, desiredReplicaCount if kg == nil { return ErrKeyspaceGroupNotExists } + if kg.IsSplitting() { + return ErrKeyspaceGroupInSplit + } exists := make(map[string]struct{}) for _, member := range kg.Members { exists[member.Address] = struct{}{} @@ -737,6 +744,9 @@ func (m *GroupManager) SetNodesForKeyspaceGroup(id uint32, nodes []string) error if kg == nil { return ErrKeyspaceGroupNotExists } + if kg.IsSplitting() { + return ErrKeyspaceGroupInSplit + } members := make([]endpoint.KeyspaceGroupMember, 0, len(nodes)) for _, node := range nodes { members = append(members, endpoint.KeyspaceGroupMember{Address: node}) diff --git a/pkg/keyspace/tso_keyspace_group_test.go b/pkg/keyspace/tso_keyspace_group_test.go index ed7992ba552..6286a71b3aa 100644 --- a/pkg/keyspace/tso_keyspace_group_test.go +++ b/pkg/keyspace/tso_keyspace_group_test.go @@ -21,6 +21,7 @@ import ( "time" "github.com/stretchr/testify/suite" + "github.com/tikv/pd/pkg/mcs/utils" "github.com/tikv/pd/pkg/mock/mockcluster" "github.com/tikv/pd/pkg/mock/mockconfig" "github.com/tikv/pd/pkg/mock/mockid" @@ -242,10 +243,14 @@ func (suite *keyspaceGroupTestSuite) TestKeyspaceGroupSplit() { ID: uint32(2), UserKind: endpoint.Standard.String(), Keyspaces: []uint32{111, 222, 333}, + Members: make([]endpoint.KeyspaceGroupMember, utils.KeyspaceGroupDefaultReplicaCount), }, } err := suite.kgm.CreateKeyspaceGroups(keyspaceGroups) re.NoError(err) + // split the keyspace group 1 to 4 + err = suite.kgm.SplitKeyspaceGroupByID(1, 4, []uint32{333}) + re.ErrorIs(err, ErrKeyspaceGroupNotEnoughReplicas) // split the keyspace group 2 to 4 err = suite.kgm.SplitKeyspaceGroupByID(2, 4, []uint32{333}) re.NoError(err) diff --git a/pkg/keyspace/util.go b/pkg/keyspace/util.go index 9b89a1e569e..69c1e776f04 100644 --- a/pkg/keyspace/util.go +++ b/pkg/keyspace/util.go @@ -51,6 +51,8 @@ var ( ErrKeyspaceGroupNotInSplit = errors.New("keyspace group is not in split state") // ErrKeyspaceNotInKeyspaceGroup is used to indicate target keyspace is not in this keyspace group. ErrKeyspaceNotInKeyspaceGroup = errors.New("keyspace is not in this keyspace group") + // ErrKeyspaceGroupNotEnoughReplicas is used to indicate not enough replicas in the keyspace group. + ErrKeyspaceGroupNotEnoughReplicas = errors.New("not enough replicas in the keyspace group") // ErrNoAvailableNode is used to indicate no available node in the keyspace group. ErrNoAvailableNode = errors.New("no available node") errModifyDefault = errors.New("cannot modify default keyspace's state") diff --git a/pkg/tso/keyspace_group_manager.go b/pkg/tso/keyspace_group_manager.go index 596c7501f71..6b1c290f1e4 100644 --- a/pkg/tso/keyspace_group_manager.go +++ b/pkg/tso/keyspace_group_manager.go @@ -578,11 +578,8 @@ func (kgm *KeyspaceGroupManager) updateKeyspaceGroup(group *endpoint.KeyspaceGro log.Info("keyspace group is in split", zap.Uint32("target", group.ID), zap.Uint32("source", splitSource)) - splitSourceAM, _ := kgm.getKeyspaceGroupMeta(splitSource) - if splitSourceAM == nil { - log.Error("the split source keyspace group is not initialized", - zap.Uint32("target", group.ID), - zap.Uint32("source", splitSource)) + splitSourceAM, splitSourceGroup := kgm.getKeyspaceGroupMeta(splitSource) + if !validateSplit(splitSourceAM, group, splitSourceGroup) { // Put the group into the retry list to retry later. kgm.groupUpdateRetryList[group.ID] = group return @@ -616,6 +613,35 @@ func (kgm *KeyspaceGroupManager) updateKeyspaceGroup(group *endpoint.KeyspaceGro kgm.Unlock() } +// validateSplit checks whether the meta info of split keyspace group +// to ensure that the split process could be continued. +func validateSplit( + sourceAM *AllocatorManager, + targetGroup, sourceGroup *endpoint.KeyspaceGroup, +) bool { + splitSourceID := targetGroup.SplitSource() + // Make sure that the split source keyspace group has been initialized. + if sourceAM == nil || sourceGroup == nil { + log.Error("the split source keyspace group is not initialized", + zap.Uint32("target", targetGroup.ID), + zap.Uint32("source", splitSourceID)) + return false + } + // Since the target group is derived from the source group and both of them + // could not be modified during the split process, so we can only check the + // member count of the source group here. + memberCount := len(sourceGroup.Members) + if memberCount < mcsutils.KeyspaceGroupDefaultReplicaCount { + log.Error("the split source keyspace group does not have enough members", + zap.Uint32("target", targetGroup.ID), + zap.Uint32("source", splitSourceID), + zap.Int("member-count", memberCount), + zap.Int("replica-count", mcsutils.KeyspaceGroupDefaultReplicaCount)) + return false + } + return true +} + // updateKeyspaceGroupMembership updates the keyspace lookup table for the given keyspace group. func (kgm *KeyspaceGroupManager) updateKeyspaceGroupMembership( oldGroup, newGroup *endpoint.KeyspaceGroup, updateWithLock bool, diff --git a/pkg/tso/keyspace_group_manager_test.go b/pkg/tso/keyspace_group_manager_test.go index c023efa02f8..390489366f9 100644 --- a/pkg/tso/keyspace_group_manager_test.go +++ b/pkg/tso/keyspace_group_manager_test.go @@ -875,11 +875,11 @@ func (suite *keyspaceGroupManagerTestSuite) TestGroupSplitUpdateRetry() { events := []*etcdEvent{} // Split target keyspace group event arrives first. - events = append(events, generateKeyspaceGroupPutEvent(2, []uint32{2}, []string{svcAddr}, &endpoint.SplitState{ + events = append(events, generateKeyspaceGroupPutEvent(2, []uint32{2} /* Mock 2 replicas */, []string{svcAddr, svcAddr}, &endpoint.SplitState{ SplitSource: 1, })) // Split source keyspace group event arrives later. - events = append(events, generateKeyspaceGroupPutEvent(1, []uint32{1}, []string{svcAddr}, &endpoint.SplitState{ + events = append(events, generateKeyspaceGroupPutEvent(1, []uint32{1}, []string{svcAddr, svcAddr}, &endpoint.SplitState{ SplitSource: 1, })) diff --git a/tests/server/apiv2/handlers/tso_keyspace_group_test.go b/tests/server/apiv2/handlers/tso_keyspace_group_test.go index a0408711e32..630b745adb1 100644 --- a/tests/server/apiv2/handlers/tso_keyspace_group_test.go +++ b/tests/server/apiv2/handlers/tso_keyspace_group_test.go @@ -137,6 +137,7 @@ func (suite *keyspaceGroupTestSuite) TestSplitKeyspaceGroup() { ID: uint32(1), UserKind: endpoint.Standard.String(), Keyspaces: []uint32{111, 222, 333}, + Members: make([]endpoint.KeyspaceGroupMember, utils.KeyspaceGroupDefaultReplicaCount), }, }}