From 5f7236fb55e6ba01bbbafbc86749ac922b351154 Mon Sep 17 00:00:00 2001
From: glorv <glorvs@163.com>
Date: Mon, 3 Jul 2023 14:53:12 +0800
Subject: [PATCH] resourcemanager: do not check existence when add resource
 group (#6717)

ref tikv/pd#5851, ref pingcap/tidb#45050

Signed-off-by: glorv <glorvs@163.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
---
 errors.toml                                          |  5 -----
 pkg/errs/errno.go                                    |  7 +++----
 pkg/mcs/resourcemanager/server/manager.go            |  8 ++------
 .../mcs/resourcemanager/resource_manager_test.go     | 12 +++++-------
 4 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/errors.toml b/errors.toml
index ed3cd32d52a..43fc6a582aa 100644
--- a/errors.toml
+++ b/errors.toml
@@ -606,11 +606,6 @@ error = '''
 invalid group settings, please check the group name, priority and the number of resources
 '''
 
-["PD:resourcemanager:ErrResourceGroupAlreadyExists"]
-error = '''
-the %s resource group already exists
-'''
-
 ["PD:schedule:ErrCreateOperator"]
 error = '''
 unable to create operator, %s
diff --git a/pkg/errs/errno.go b/pkg/errs/errno.go
index 0fda57fa7c9..0bd2a57dba5 100644
--- a/pkg/errs/errno.go
+++ b/pkg/errs/errno.go
@@ -372,8 +372,7 @@ var (
 
 // Resource Manager errors
 var (
-	ErrResourceGroupAlreadyExists = errors.Normalize("the %s resource group already exists", errors.RFCCodeText("PD:resourcemanager:ErrResourceGroupAlreadyExists"))
-	ErrResourceGroupNotExists     = errors.Normalize("the %s resource group does not exist", errors.RFCCodeText("PD:resourcemanager:ErrGroupNotExists"))
-	ErrDeleteReservedGroup        = errors.Normalize("cannot delete reserved group", errors.RFCCodeText("PD:resourcemanager:ErrDeleteReservedGroup"))
-	ErrInvalidGroup               = errors.Normalize("invalid group settings, please check the group name, priority and the number of resources", errors.RFCCodeText("PD:resourcemanager:ErrInvalidGroup"))
+	ErrResourceGroupNotExists = errors.Normalize("the %s resource group does not exist", errors.RFCCodeText("PD:resourcemanager:ErrGroupNotExists"))
+	ErrDeleteReservedGroup    = errors.Normalize("cannot delete reserved group", errors.RFCCodeText("PD:resourcemanager:ErrDeleteReservedGroup"))
+	ErrInvalidGroup           = errors.Normalize("invalid group settings, please check the group name, priority and the number of resources", errors.RFCCodeText("PD:resourcemanager:ErrInvalidGroup"))
 )
diff --git a/pkg/mcs/resourcemanager/server/manager.go b/pkg/mcs/resourcemanager/server/manager.go
index a98d274f506..b054a37e0ac 100644
--- a/pkg/mcs/resourcemanager/server/manager.go
+++ b/pkg/mcs/resourcemanager/server/manager.go
@@ -154,6 +154,8 @@ func (m *Manager) Init(ctx context.Context) {
 }
 
 // AddResourceGroup puts a resource group.
+// NOTE: AddResourceGroup should also be idempotent because tidb depends
+// on this retry mechanism.
 func (m *Manager) AddResourceGroup(grouppb *rmpb.ResourceGroup) error {
 	// Check the name.
 	if len(grouppb.Name) == 0 || len(grouppb.Name) > 32 {
@@ -163,12 +165,6 @@ func (m *Manager) AddResourceGroup(grouppb *rmpb.ResourceGroup) error {
 	if grouppb.GetPriority() > 16 {
 		return errs.ErrInvalidGroup
 	}
-	m.RLock()
-	_, ok := m.groups[grouppb.Name]
-	m.RUnlock()
-	if ok {
-		return errs.ErrResourceGroupAlreadyExists.FastGenByArgs(grouppb.Name)
-	}
 	group := FromProtoResourceGroup(grouppb)
 	m.Lock()
 	defer m.Unlock()
diff --git a/tests/integrations/mcs/resourcemanager/resource_manager_test.go b/tests/integrations/mcs/resourcemanager/resource_manager_test.go
index 467eba6c518..5cbed81d57e 100644
--- a/tests/integrations/mcs/resourcemanager/resource_manager_test.go
+++ b/tests/integrations/mcs/resourcemanager/resource_manager_test.go
@@ -711,7 +711,7 @@ func (suite *resourceManagerClientTestSuite) TestBasicResourceGroupCURD() {
 	testCasesSet1 := []struct {
 		name           string
 		mode           rmpb.GroupMode
-		addSuccess     bool
+		isNewGroup     bool
 		modifySuccess  bool
 		expectMarshal  string
 		modifySettings func(*rmpb.ResourceGroup)
@@ -789,8 +789,8 @@ func (suite *resourceManagerClientTestSuite) TestBasicResourceGroupCURD() {
 		}
 		// Create Resource Group
 		resp, err := cli.AddResourceGroup(suite.ctx, group)
-		checkErr(err, tcase.addSuccess)
-		if tcase.addSuccess {
+		checkErr(err, true)
+		if tcase.isNewGroup {
 			finalNum++
 			re.Contains(resp, "Success!")
 		}
@@ -860,11 +860,9 @@ func (suite *resourceManagerClientTestSuite) TestBasicResourceGroupCURD() {
 		resp, err := http.Post(getAddr(i)+"/resource-manager/api/v1/config/group", "application/json", strings.NewReader(string(createJSON)))
 		re.NoError(err)
 		defer resp.Body.Close()
-		if tcase.addSuccess {
-			re.Equal(http.StatusOK, resp.StatusCode)
+		re.Equal(http.StatusOK, resp.StatusCode)
+		if tcase.isNewGroup {
 			finalNum++
-		} else {
-			re.Equal(http.StatusInternalServerError, resp.StatusCode)
 		}
 
 		// Modify Resource Group