You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by lu...@apache.org on 2023/04/27 02:48:23 UTC

[doris] branch master updated: [Feature](resource-group) Support alter resource group (#18990)

This is an automated email from the ASF dual-hosted git repository.

luozenglin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new 908518915d [Feature](resource-group) Support alter resource group (#18990)
908518915d is described below

commit 908518915dc7a1934f817cc52af14e64c87d5186
Author: 赵立伟 <zh...@xiaomi.com>
AuthorDate: Thu Apr 27 10:48:17 2023 +0800

    [Feature](resource-group) Support alter resource group (#18990)
---
 fe/fe-core/src/main/cup/sql_parser.cup             |  4 ++
 .../doris/analysis/AlterResourceGroupStmt.java     | 69 ++++++++++++++++++++++
 .../org/apache/doris/journal/JournalEntity.java    |  3 +-
 .../java/org/apache/doris/persist/EditLog.java     |  9 +++
 .../org/apache/doris/persist/OperationType.java    |  1 +
 .../main/java/org/apache/doris/qe/DdlExecutor.java |  3 +
 .../resource/resourcegroup/ResourceGroup.java      | 28 +++++++++
 .../resource/resourcegroup/ResourceGroupMgr.java   | 54 +++++++++++++----
 .../resourcegroup/ResourceGroupMgrTest.java        | 29 +++++++++
 9 files changed, 188 insertions(+), 12 deletions(-)

diff --git a/fe/fe-core/src/main/cup/sql_parser.cup b/fe/fe-core/src/main/cup/sql_parser.cup
index 8418cf58ed..e563d89bcf 100644
--- a/fe/fe-core/src/main/cup/sql_parser.cup
+++ b/fe/fe-core/src/main/cup/sql_parser.cup
@@ -1327,6 +1327,10 @@ alter_stmt ::=
     {:
         RESULT = new AlterResourceStmt(resourceName, properties);
     :}
+    | KW_ALTER KW_RESOURCE KW_GROUP ident_or_text:resourceGroupName opt_properties:properties
+    {:
+        RESULT = new AlterResourceGroupStmt(resourceGroupName, properties);
+    :}
     | KW_ALTER KW_ROUTINE KW_LOAD KW_FOR job_label:jobLabel opt_properties:jobProperties
       opt_datasource_properties:datasourceProperties
     {:
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterResourceGroupStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterResourceGroupStmt.java
new file mode 100644
index 0000000000..bbec596b66
--- /dev/null
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterResourceGroupStmt.java
@@ -0,0 +1,69 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.analysis;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.ErrorCode;
+import org.apache.doris.common.ErrorReport;
+import org.apache.doris.common.UserException;
+import org.apache.doris.common.util.PrintableMap;
+import org.apache.doris.mysql.privilege.PrivPredicate;
+import org.apache.doris.qe.ConnectContext;
+
+import java.util.Map;
+
+public class AlterResourceGroupStmt extends DdlStmt {
+    private final String resourceGroupName;
+    private final Map<String, String> properties;
+
+    public AlterResourceGroupStmt(String resourceGroupName, Map<String, String> properties) {
+        this.resourceGroupName = resourceGroupName;
+        this.properties = properties;
+    }
+
+    public String getResourceGroupName() {
+        return resourceGroupName;
+    }
+
+    public Map<String, String> getProperties() {
+        return properties;
+    }
+
+    @Override
+    public void analyze(Analyzer analyzer) throws UserException {
+        super.analyze(analyzer);
+
+        // check auth
+        if (!Env.getCurrentEnv().getAccessManager().checkGlobalPriv(ConnectContext.get(), PrivPredicate.ADMIN)) {
+            ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "ADMIN");
+        }
+
+        if (properties == null || properties.isEmpty()) {
+            throw new AnalysisException("Resource group properties can't be null");
+        }
+    }
+
+    @Override
+    public String toSql() {
+        StringBuilder sb = new StringBuilder();
+        sb.append("ALTER RESOURCE GROUP '").append(resourceGroupName).append("' ");
+        sb.append("PROPERTIES(").append(new PrintableMap<>(properties, " = ", true, false)).append(")");
+        return sb.toString();
+    }
+}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/journal/JournalEntity.java b/fe/fe-core/src/main/java/org/apache/doris/journal/JournalEntity.java
index 50aeb15080..9cbcdfb2d5 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/journal/JournalEntity.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/journal/JournalEntity.java
@@ -803,7 +803,8 @@ public class JournalEntity implements Writable {
                 isRead = true;
                 break;
             }
-            case OperationType.OP_CREATE_RESOURCE_GROUP: {
+            case OperationType.OP_CREATE_RESOURCE_GROUP:
+            case OperationType.OP_ALTER_RESOURCE_GROUP: {
                 data = ResourceGroup.read(in);
                 isRead = true;
                 break;
diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java b/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
index b20f5a3be7..094087f4fe 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
@@ -1018,6 +1018,11 @@ public class EditLog {
                     env.getResourceGroupMgr().replayDropResourceGroup(operationLog);
                     break;
                 }
+                case OperationType.OP_ALTER_RESOURCE_GROUP: {
+                    final ResourceGroup resource = (ResourceGroup) journal.getData();
+                    env.getResourceGroupMgr().replayAlterResourceGroup(resource);
+                    break;
+                }
                 case OperationType.OP_INIT_EXTERNAL_TABLE: {
                     // Do nothing.
                     break;
@@ -1564,6 +1569,10 @@ public class EditLog {
         logEdit(OperationType.OP_ALTER_RESOURCE, resource);
     }
 
+    public void logAlterResourceGroup(ResourceGroup resourceGroup) {
+        logEdit(OperationType.OP_ALTER_RESOURCE_GROUP, resourceGroup);
+    }
+
     public void logCreateResourceGroup(ResourceGroup resourceGroup) {
         logEdit(OperationType.OP_CREATE_RESOURCE_GROUP, resourceGroup);
     }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/OperationType.java b/fe/fe-core/src/main/java/org/apache/doris/persist/OperationType.java
index d143268a08..fcdfdfe628 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/persist/OperationType.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/persist/OperationType.java
@@ -284,6 +284,7 @@ public class OperationType {
     // resource group 410 ~ 419
     public static final short OP_CREATE_RESOURCE_GROUP = 410;
     public static final short OP_DROP_RESOURCE_GROUP = 411;
+    public static final short OP_ALTER_RESOURCE_GROUP = 412;
 
     /**
      * Get opcode name by op code.
diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/DdlExecutor.java b/fe/fe-core/src/main/java/org/apache/doris/qe/DdlExecutor.java
index cb12c3fdb7..cfdbf467c3 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/qe/DdlExecutor.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/qe/DdlExecutor.java
@@ -35,6 +35,7 @@ import org.apache.doris.analysis.AlterDatabaseQuotaStmt;
 import org.apache.doris.analysis.AlterDatabaseRename;
 import org.apache.doris.analysis.AlterMaterializedViewStmt;
 import org.apache.doris.analysis.AlterPolicyStmt;
+import org.apache.doris.analysis.AlterResourceGroupStmt;
 import org.apache.doris.analysis.AlterResourceStmt;
 import org.apache.doris.analysis.AlterRoutineLoadStmt;
 import org.apache.doris.analysis.AlterSqlBlockRuleStmt;
@@ -303,6 +304,8 @@ public class DdlExecutor {
             env.createAnalysisJob((AnalyzeStmt) ddlStmt);
         } else if (ddlStmt instanceof AlterResourceStmt) {
             env.getResourceMgr().alterResource((AlterResourceStmt) ddlStmt);
+        } else if (ddlStmt instanceof AlterResourceGroupStmt) {
+            env.getResourceGroupMgr().alterResourceGroup((AlterResourceGroupStmt) ddlStmt);
         } else if (ddlStmt instanceof CreatePolicyStmt) {
             env.getPolicyMgr().createPolicy((CreatePolicyStmt) ddlStmt);
         } else if (ddlStmt instanceof DropPolicyStmt) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/resource/resourcegroup/ResourceGroup.java b/fe/fe-core/src/main/java/org/apache/doris/resource/resourcegroup/ResourceGroup.java
index 2c3dfb4caa..538c064247 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/resource/resourcegroup/ResourceGroup.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/resource/resourcegroup/ResourceGroup.java
@@ -25,6 +25,7 @@ import org.apache.doris.common.proc.BaseProcResult;
 import org.apache.doris.persist.gson.GsonUtils;
 import org.apache.doris.thrift.TPipelineResourceGroup;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.gson.annotations.SerializedName;
@@ -33,6 +34,7 @@ import org.apache.commons.lang3.StringUtils;
 import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.Map;
 
 public class ResourceGroup implements Writable {
@@ -65,11 +67,33 @@ public class ResourceGroup implements Writable {
         this.version = 0;
     }
 
+    private ResourceGroup(long id, String name, Map<String, String> properties, long version) {
+        this.id = id;
+        this.name = name;
+        this.properties = properties;
+        this.version = version;
+    }
+
     public static ResourceGroup create(String name, Map<String, String> properties) throws DdlException {
         checkProperties(properties);
         return new ResourceGroup(Env.getCurrentEnv().getNextId(), name, properties);
     }
 
+    public static ResourceGroup create(ResourceGroup resourceGroup, Map<String, String> updateProperties)
+            throws DdlException {
+        Map<String, String> newProperties = new HashMap<>();
+        newProperties.putAll(resourceGroup.getProperties());
+        for (Map.Entry<String, String> kv : updateProperties.entrySet()) {
+            if (!Strings.isNullOrEmpty(kv.getValue())) {
+                newProperties.put(kv.getKey(), kv.getValue());
+            }
+        }
+
+        checkProperties(newProperties);
+        return new ResourceGroup(
+           resourceGroup.getId(), resourceGroup.getName(), newProperties, resourceGroup.getVersion() + 1);
+    }
+
     private static void checkProperties(Map<String, String> properties) throws DdlException {
         for (String propertyName : properties.keySet()) {
             if (!ALL_PROPERTIES_NAME.contains(propertyName)) {
@@ -100,6 +124,10 @@ public class ResourceGroup implements Writable {
         return properties;
     }
 
+    private long getVersion() {
+        return version;
+    }
+
     public void getProcNodeData(BaseProcResult result) {
         for (Map.Entry<String, String> entry : properties.entrySet()) {
             result.addRow(Lists.newArrayList(String.valueOf(id), name, entry.getKey(), entry.getValue()));
diff --git a/fe/fe-core/src/main/java/org/apache/doris/resource/resourcegroup/ResourceGroupMgr.java b/fe/fe-core/src/main/java/org/apache/doris/resource/resourcegroup/ResourceGroupMgr.java
index 20f24ad315..d4e824365c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/resource/resourcegroup/ResourceGroupMgr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/resource/resourcegroup/ResourceGroupMgr.java
@@ -17,6 +17,7 @@
 
 package org.apache.doris.resource.resourcegroup;
 
+import org.apache.doris.analysis.AlterResourceGroupStmt;
 import org.apache.doris.analysis.CreateResourceGroupStmt;
 import org.apache.doris.analysis.DropResourceGroupStmt;
 import org.apache.doris.catalog.Env;
@@ -85,6 +86,12 @@ public class ResourceGroupMgr implements Writable, GsonPostProcessable {
         lock.writeLock().unlock();
     }
 
+    private void checkResourceGroupEnabled() throws DdlException {
+        if (!Config.enable_resource_group) {
+            throw new DdlException("unsupported feature now, coming soon.");
+        }
+    }
+
     public void init() {
         if (Config.enable_resource_group || Config.use_fuzzy_session_variable /* for github workflow */) {
             checkAndCreateDefaultGroup();
@@ -99,7 +106,6 @@ public class ResourceGroupMgr implements Writable, GsonPostProcessable {
             if (resourceGroup == null) {
                 throw new UserException("Resource group " + groupName + " does not exist");
             }
-            // need to check resource group privs
             resourceGroups.add(resourceGroup.toThrift());
         } finally {
             readUnlock();
@@ -129,19 +135,17 @@ public class ResourceGroupMgr implements Writable, GsonPostProcessable {
     }
 
     public void createResourceGroup(CreateResourceGroupStmt stmt) throws DdlException {
-        if (!Config.enable_resource_group) {
-            throw new DdlException("unsupported feature now, coming soon.");
-        }
+        checkResourceGroupEnabled();
 
         ResourceGroup resourceGroup = ResourceGroup.create(stmt.getResourceGroupName(), stmt.getProperties());
-        String resourceGroupNameName = resourceGroup.getName();
+        String resourceGroupName = resourceGroup.getName();
         writeLock();
         try {
-            if (nameToResourceGroup.putIfAbsent(resourceGroupNameName, resourceGroup) != null) {
+            if (nameToResourceGroup.putIfAbsent(resourceGroupName, resourceGroup) != null) {
                 if (stmt.isIfNotExists()) {
                     return;
                 }
-                throw new DdlException("Resource group " + resourceGroupNameName + " already exist");
+                throw new DdlException("Resource group " + resourceGroupName + " already exist");
             }
             idToResourceGroup.put(resourceGroup.getId(), resourceGroup);
             Env.getCurrentEnv().getEditLog().logCreateResourceGroup(resourceGroup);
@@ -151,10 +155,30 @@ public class ResourceGroupMgr implements Writable, GsonPostProcessable {
         LOG.info("Create resource group success: {}", resourceGroup);
     }
 
-    public void dropResourceGroup(DropResourceGroupStmt stmt) throws DdlException {
-        if (!Config.enable_resource_group) {
-            throw new DdlException("unsupported feature now, coming soon.");
+    public void alterResourceGroup(AlterResourceGroupStmt stmt) throws DdlException {
+        checkResourceGroupEnabled();
+
+        String resourceGroupName = stmt.getResourceGroupName();
+        Map<String, String> properties = stmt.getProperties();
+        ResourceGroup newResourceGroup;
+        writeLock();
+        try {
+            if (!nameToResourceGroup.containsKey(resourceGroupName)) {
+                throw new DdlException("Resource Group(" + resourceGroupName + ") does not exist.");
+            }
+            ResourceGroup resourceGroup = nameToResourceGroup.get(resourceGroupName);
+            newResourceGroup = ResourceGroup.create(resourceGroup, properties);
+            nameToResourceGroup.put(resourceGroupName, newResourceGroup);
+            idToResourceGroup.put(newResourceGroup.getId(), newResourceGroup);
+            Env.getCurrentEnv().getEditLog().logAlterResourceGroup(newResourceGroup);
+        } finally {
+            writeUnlock();
         }
+        LOG.info("Alter resource success: {}", newResourceGroup);
+    }
+
+    public void dropResourceGroup(DropResourceGroupStmt stmt) throws DdlException {
+        checkResourceGroupEnabled();
 
         String resourceGroupName = stmt.getResourceGroupName();
         if (resourceGroupName == DEFAULT_GROUP_NAME) {
@@ -180,7 +204,7 @@ public class ResourceGroupMgr implements Writable, GsonPostProcessable {
         LOG.info("Drop resource group success: {}", resourceGroupName);
     }
 
-    public void replayCreateResourceGroup(ResourceGroup resourceGroup) {
+    private void insertResourceGroup(ResourceGroup resourceGroup) {
         writeLock();
         try {
             nameToResourceGroup.put(resourceGroup.getName(), resourceGroup);
@@ -190,6 +214,14 @@ public class ResourceGroupMgr implements Writable, GsonPostProcessable {
         }
     }
 
+    public void replayCreateResourceGroup(ResourceGroup resourceGroup) {
+        insertResourceGroup(resourceGroup);
+    }
+
+    public void replayAlterResourceGroup(ResourceGroup resourceGroup) {
+        insertResourceGroup(resourceGroup);
+    }
+
     public void replayDropResourceGroup(DropResourceGroupOperationLog operationLog) {
         long id = operationLog.getId();
         writeLock();
diff --git a/fe/fe-core/src/test/java/org/apache/doris/resource/resourcegroup/ResourceGroupMgrTest.java b/fe/fe-core/src/test/java/org/apache/doris/resource/resourcegroup/ResourceGroupMgrTest.java
index ac431b748c..4a0f18914a 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/resource/resourcegroup/ResourceGroupMgrTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/resource/resourcegroup/ResourceGroupMgrTest.java
@@ -17,6 +17,7 @@
 
 package org.apache.doris.resource.resourcegroup;
 
+import org.apache.doris.analysis.AlterResourceGroupStmt;
 import org.apache.doris.analysis.CreateResourceGroupStmt;
 import org.apache.doris.analysis.DropResourceGroupStmt;
 import org.apache.doris.catalog.Env;
@@ -172,4 +173,32 @@ public class ResourceGroupMgrTest {
             Assert.assertTrue(e.getMessage().contains("is not allowed"));
         }
     }
+
+    @Test
+    public void testAlterResourceGroup() throws UserException {
+        Config.enable_resource_group = true;
+        ResourceGroupMgr resourceGroupMgr = new ResourceGroupMgr();
+        Map<String, String> properties = Maps.newHashMap();
+        String name = "g1";
+        try {
+            AlterResourceGroupStmt stmt1 = new AlterResourceGroupStmt(name, properties);
+            resourceGroupMgr.alterResourceGroup(stmt1);
+        } catch (DdlException e) {
+            Assert.assertTrue(e.getMessage().contains("does not exist"));
+        }
+
+        properties.put(ResourceGroup.CPU_SHARE, "10");
+        CreateResourceGroupStmt createStmt = new CreateResourceGroupStmt(false, name, properties);
+        resourceGroupMgr.createResourceGroup(createStmt);
+
+        Map<String, String> newProperties = Maps.newHashMap();
+        newProperties.put(ResourceGroup.CPU_SHARE, "5");
+        AlterResourceGroupStmt stmt2 = new AlterResourceGroupStmt(name, newProperties);
+        resourceGroupMgr.alterResourceGroup(stmt2);
+
+        List<TPipelineResourceGroup> tResourceGroups = resourceGroupMgr.getResourceGroup(name);
+        Assert.assertEquals(1, tResourceGroups.size());
+        TPipelineResourceGroup tResourceGroup1 = tResourceGroups.get(0);
+        Assert.assertEquals(tResourceGroup1.getProperties().get(ResourceGroup.CPU_SHARE), "5");
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org