[BugFix] Make cpu_weight always store positive default value (StarRoc…

…ks#51005) Signed-off-by: zihe.liu <[email protected]> Signed-off-by: zhiminr.ren <[email protected]>
renzhimin7 · Nov 7, 2024 · 7f85a79 · 7f85a79
1 parent 9cf78f2
commit 7f85a79
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 10 deletions.
diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/ResourceGroup.java b/fe/fe-core/src/main/java/com/starrocks/catalog/ResourceGroup.java
@@ -89,7 +89,7 @@ public ColumnMeta(Column column, BiFunction<ResourceGroup, ResourceGroupClassifi
                     (rg, classifier) -> "" + rg.getId()),
             new ColumnMeta(
                     new Column(CPU_WEIGHT, ScalarType.createVarchar(200)),
-                    (rg, classifier) -> "" + rg.getCpuWeight()),
+                    (rg, classifier) -> "" + rg.geNormalizedCpuWeight()),
             new ColumnMeta(
                     new Column(EXCLUSIVE_CPU_CORES, ScalarType.createVarchar(200)),
                     (rg, classifier) -> "" + rg.getNormalizedExclusiveCpuCores()),
@@ -264,18 +264,35 @@ public TWorkGroup toThrift() {
         return twg;
     }
 
-    public Integer getCpuWeight() {
+    public Integer getRawCpuWeight() {
         return cpuWeight;
     }
 
     public void setCpuWeight(int cpuWeight) {
         this.cpuWeight = cpuWeight;
     }
 
+    public int geNormalizedCpuWeight() {
+        if (exclusiveCpuCores != null && exclusiveCpuCores > 0) {
+            return 0;
+        }
+        return cpuWeight;
+    }
+
+    /**
+     * The old version considers cpu_weight as a positive integer, but now it can be non-positive.
+     * To be compatible with the old version, if cpu_weight is non-positive, it is stored as 1.
+     * And use geNormalizedCpuWeight() to get the normalized value when using cpu_weight.
+     */
+    public void normalizeCpuWeight() {
+        if (cpuWeight == null || cpuWeight <= 0) {
+            cpuWeight = 1;
+        }
+    }
+
     public Integer getExclusiveCpuCores() {
         return exclusiveCpuCores;
     }
-
     public int getNormalizedExclusiveCpuCores() {
         if (exclusiveCpuCores != null && exclusiveCpuCores > 0) {
             return exclusiveCpuCores;

diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/ResourceGroupMgr.java b/fe/fe-core/src/main/java/com/starrocks/catalog/ResourceGroupMgr.java
@@ -148,9 +148,7 @@ public void createResourceGroup(CreateResourceGroupStmt stmt) throws DdlExceptio
                 dropResourceGroupUnlocked(wg.getName());
             }
 
-            if (wg.getCpuWeight() == null) {
-                wg.setCpuWeight(0);
-            }
+            wg.normalizeCpuWeight();
 
             if (ResourceGroup.DEFAULT_RESOURCE_GROUP_NAME.equals(wg.getName())) {
                 wg.setId(ResourceGroup.DEFAULT_WG_ID);
@@ -350,9 +348,9 @@ public void alterResourceGroup(AlterResourceGroupStmt stmt) throws DdlException
             } else if (cmd instanceof AlterResourceGroupStmt.AlterProperties) {
                 ResourceGroup changedProperties = stmt.getChangedProperties();
 
-                Integer cpuWeight = changedProperties.getCpuWeight();
+                Integer cpuWeight = changedProperties.getRawCpuWeight();
                 if (cpuWeight == null) {
-                    cpuWeight = wg.getCpuWeight();
+                    cpuWeight = wg.geNormalizedCpuWeight();
                 }
                 Integer exclusiveCpuCores = changedProperties.getExclusiveCpuCores();
                 if (exclusiveCpuCores == null) {
@@ -377,6 +375,7 @@ public void alterResourceGroup(AlterResourceGroupStmt stmt) throws DdlException
                 if (cpuWeight != null) {
                     wg.setCpuWeight(cpuWeight);
                 }
+                wg.normalizeCpuWeight();
 
                 if (exclusiveCpuCores != null) {
                     sumExclusiveCpuCores -= wg.getNormalizedExclusiveCpuCores();

diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/ast/AlterResourceGroupStmt.java b/fe/fe-core/src/main/java/com/starrocks/sql/ast/AlterResourceGroupStmt.java
@@ -77,7 +77,7 @@ public void analyze() {
             if (changedProperties.getResourceGroupType() != null) {
                 throw new SemanticException("type of ResourceGroup is immutable");
             }
-            if (changedProperties.getCpuWeight() == null &&
+            if (changedProperties.getRawCpuWeight() == null &&
                     changedProperties.getExclusiveCpuCores() == null &&
                     changedProperties.getMemLimit() == null &&
                     changedProperties.getConcurrencyLimit() == null &&

diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/ast/CreateResourceGroupStmt.java b/fe/fe-core/src/main/java/com/starrocks/sql/ast/CreateResourceGroupStmt.java
@@ -95,7 +95,7 @@ public void analyze() throws SemanticException {
             throw new SemanticException(SHORT_QUERY_SET_EXCLUSIVE_CPU_CORES_ERR_MSG);
         }
 
-        ResourceGroup.validateCpuParameters(resourceGroup.getCpuWeight(), resourceGroup.getExclusiveCpuCores());
+        ResourceGroup.validateCpuParameters(resourceGroup.getRawCpuWeight(), resourceGroup.getExclusiveCpuCores());
 
         if (resourceGroup.getMemLimit() == null) {
             throw new SemanticException("property 'mem_limit' is absent");