add getBackgroundSettings and add test

Signed-off-by: husharp <[email protected]>
HuSharp · Jul 19, 2023 · a233a44 · a233a44
1 parent 6698aa7
commit a233a44
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 18 deletions.
diff --git a/client/resource_group/controller/controller.go b/client/resource_group/controller/controller.go
@@ -484,9 +484,7 @@ func (c *ResourceGroupsController) IsBackgroundRequest(ctx context.Context,
 		return false
 	}
 
-	gc.metaLock.RLock()
-	defer gc.metaLock.RUnlock()
-	return c.checkBackgroundSettings(ctx, gc.meta.BackgroundSettings, requestResource)
+	return c.checkBackgroundSettings(ctx, gc.getBackgroundSettings(), requestResource)
 }
 
 func (c *ResourceGroupsController) checkBackgroundSettings(ctx context.Context, bg *rmpb.BackgroundSettings, requestResource string) bool {
@@ -498,7 +496,7 @@ func (c *ResourceGroupsController) checkBackgroundSettings(ctx context.Context,
 			failedRequestCounter.WithLabelValues(resourceGroupName).Inc()
 			return false
 		}
-		bg = gc.meta.BackgroundSettings
+		bg = gc.getBackgroundSettings()
 	}
 
 	if bg == nil || len(requestResource) == 0 || len(bg.JobTypes) == 0 {
@@ -1084,6 +1082,12 @@ func (gc *groupCostController) collectRequestAndConsumption(selectTyp selectType
 	return req
 }
 
+func (gc *groupCostController) getBackgroundSettings() *rmpb.BackgroundSettings {
+	gc.metaLock.RLock()
+	defer gc.metaLock.RUnlock()
+	return gc.meta.BackgroundSettings
+}
+
 func (gc *groupCostController) getMeta() *rmpb.ResourceGroup {
 	gc.metaLock.Lock()
 	defer gc.metaLock.Unlock()

diff --git a/pkg/mcs/resourcemanager/server/resource_group.go b/pkg/mcs/resourcemanager/server/resource_group.go
@@ -106,7 +106,6 @@ func (rg *ResourceGroup) PatchSettings(metaGroup *rmpb.ResourceGroup) error {
 			return errors.New("invalid resource group settings, RU mode should set RU settings")
 		}
 		rg.RUSettings.RU.patch(settings.GetRU())
-		log.Info("patch resource group ru settings", zap.String("name", rg.Name), zap.Any("settings", settings))
 	case rmpb.GroupMode_RawMode:
 		panic("no implementation")
 	}

diff --git a/tests/integrations/mcs/resourcemanager/resource_manager_test.go b/tests/integrations/mcs/resourcemanager/resource_manager_test.go
@@ -1189,6 +1189,30 @@ func (suite *resourceManagerClientTestSuite) TestSkipConsumptionForBackgroundJob
 		re.Contains(resp, "Success!")
 	}
 
+	cfg := &controller.RequestUnitConfig{
+		ReadBaseCost:     1,
+		ReadCostPerByte:  1,
+		WriteBaseCost:    1,
+		WriteCostPerByte: 1,
+		CPUMsCost:        1,
+	}
+	c, _ := controller.NewResourceGroupController(suite.ctx, 1, cli, cfg)
+	c.Start(suite.ctx)
+
+	resourceGroupName := suite.initGroups[1].Name
+	re.False(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_default"))
+	// test fallback for nil.
+	re.False(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_lightning"))
+	re.False(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_ddl"))
+	re.False(c.IsBackgroundRequest(suite.ctx, resourceGroupName, ""))
+
+	resourceGroupName = "background_job"
+	re.True(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_br"))
+	re.True(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_lightning"))
+	// test fallback for nil.
+	re.False(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_ddl"))
+
+	// modify `Default` to check fallback.
 	resp, err := cli.ModifyResourceGroup(suite.ctx, &rmpb.ResourceGroup{
 		Name: "default",
 		Mode: rmpb.GroupMode_RUMode,
@@ -1205,27 +1229,20 @@ func (suite *resourceManagerClientTestSuite) TestSkipConsumptionForBackgroundJob
 	})
 	re.NoError(err)
 	re.Contains(resp, "Success!")
+	// wait for watch event modify.
+	time.Sleep(time.Millisecond * 100)
 
-	cfg := &controller.RequestUnitConfig{
-		ReadBaseCost:     1,
-		ReadCostPerByte:  1,
-		WriteBaseCost:    1,
-		WriteCostPerByte: 1,
-		CPUMsCost:        1,
-	}
-	c, _ := controller.NewResourceGroupController(suite.ctx, 1, cli, cfg)
-	c.Start(suite.ctx)
-
-	resourceGroupName := suite.initGroups[1].Name
+	resourceGroupName = suite.initGroups[1].Name
 	re.False(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_default"))
-	// test fallback.
+	// test fallback for `"lightning", "ddl"`.
 	re.True(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_lightning"))
 	re.True(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_ddl"))
+	re.False(c.IsBackgroundRequest(suite.ctx, resourceGroupName, ""))
 
 	resourceGroupName = "background_job"
 	re.True(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_br"))
 	re.True(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_lightning"))
-	// test fallback.
+	// test fallback for `"lightning", "ddl"`.
 	re.False(c.IsBackgroundRequest(suite.ctx, resourceGroupName, "internal_ddl"))
 
 	c.Stop()