caishunfeng commented on a change in pull request #6722: URL: https://github.com/apache/dolphinscheduler/pull/6722#discussion_r758101249
########## File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskGroupServiceImpl.java ########## @@ -0,0 +1,300 @@ +/* + * 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.dolphinscheduler.api.service.impl; + +import org.apache.dolphinscheduler.api.enums.Status; +import org.apache.dolphinscheduler.api.service.TaskGroupQueueService; +import org.apache.dolphinscheduler.api.service.TaskGroupService; +import org.apache.dolphinscheduler.api.utils.PageInfo; +import org.apache.dolphinscheduler.common.Constants; +import org.apache.dolphinscheduler.common.enums.Flag; +import org.apache.dolphinscheduler.dao.entity.TaskGroup; +import org.apache.dolphinscheduler.dao.entity.User; +import org.apache.dolphinscheduler.dao.mapper.TaskGroupMapper; +import org.apache.dolphinscheduler.service.process.ProcessService; +import org.apache.dolphinscheduler.spi.utils.StringUtils; + +import java.util.ArrayList; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +import com.baomidou.mybatisplus.core.metadata.IPage; +import com.baomidou.mybatisplus.extension.plugins.pagination.Page; + +/** + * task Group Service + */ +@Service +public class TaskGroupServiceImpl extends BaseServiceImpl implements TaskGroupService { + + @Autowired + private TaskGroupMapper taskGroupMapper; + + @Autowired + private TaskGroupQueueService taskGroupQueueService; + + @Autowired + private ProcessService processService; + + private static final Logger logger = LoggerFactory.getLogger(TaskGroupServiceImpl.class); + + /** + * create a Task group + * + * @param loginUser login user + * @param name task group name + * @param description task group description + * @param groupSize task group total size + * @return the result code and msg + */ + @Override + public Map<String, Object> createTaskGroup(User loginUser, String name, String description, int groupSize) { + Map<String, Object> result = new HashMap<>(); + if (isNotAdmin(loginUser, result)) { + return result; + } + if (name == null) { + putMsg(result, Status.NAME_NULL); + return result; + } + if (groupSize <= 0) { + putMsg(result, Status.TASK_GROUP_SIZE_ERROR); + return result; + + } + TaskGroup taskGroup1 = taskGroupMapper.queryByName(loginUser.getId(), name); + if (taskGroup1 != null) { + putMsg(result, Status.TASK_GROUP_NAME_EXSIT); + return result; + } + TaskGroup taskGroup = new TaskGroup(name, description, + groupSize, loginUser.getId()); + int insert = taskGroupMapper.insert(taskGroup); + logger.info("insert result:{}", insert); + putMsg(result, Status.SUCCESS); + + return result; + } + + /** + * update the task group + * + * @param loginUser login user + * @param name task group name + * @param description task group description + * @param groupSize task group total size + * @return the result code and msg + */ + @Override + public Map<String, Object> updateTaskGroup(User loginUser, int id, String name, String description, int groupSize) { + Map<String, Object> result = new HashMap<>(); + if (isNotAdmin(loginUser, result)) { + return result; + } + TaskGroup taskGroup = taskGroupMapper.selectById(id); + if (taskGroup.getStatus() != Flag.NO.getCode()) { + putMsg(result, Status.TASK_GROUP_STATUS_ERROR); + return result; + } + taskGroup.setGroupSize(groupSize); + taskGroup.setDescription(description); + if (StringUtils.isNotEmpty(name)) { + taskGroup.setName(name); + } + int i = taskGroupMapper.updateById(taskGroup); + logger.info("update result:{}", i); + putMsg(result, Status.SUCCESS); + return result; + } + + /** + * get task group status + * + * @param id task group id + * @return is the task group available + */ + @Override + public boolean isTheTaskGroupAvailable(int id) { + TaskGroup taskGroup = taskGroupMapper.selectById(id); Review comment: It's better to use `select count(1) from task_group where id = {}` instead of selecting a full row. ########## File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.xml ########## @@ -0,0 +1,87 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<!DOCTYPE mapper PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN" "http://mybatis.org/dtd/mybatis-3-mapper.dtd";> +<mapper namespace="org.apache.dolphinscheduler.dao.mapper.TaskGroupMapper"> + + <resultMap type="org.apache.dolphinscheduler.dao.entity.TaskGroup" id="TaskGroupMap"> + <result property="id" column="id" jdbcType="INTEGER"/> + <result property="name" column="name" jdbcType="VARCHAR"/> + <result property="description" column="description" jdbcType="VARCHAR"/> + <result property="groupSize" column="group_size" jdbcType="INTEGER"/> + <result property="useSize" column="use_size" jdbcType="INTEGER"/> + <result property="userId" column="user_id" jdbcType="INTEGER"/> + <result property="status" column="status" jdbcType="VARCHAR"/> + <result property="createTime" column="create_time" jdbcType="TIMESTAMP"/> + <result property="updateTime" column="update_time" jdbcType="TIMESTAMP"/> + </resultMap> + + <sql id = "baseSql"> + id,name,description,group_size,use_size,create_time,update_time + </sql> + + <select id="queryTaskGroupPaging" resultType="org.apache.dolphinscheduler.dao.entity.TaskGroup"> + select + <include refid="baseSql"> + </include> + from t_ds_task_group + <where> + <if test="userId != 0"> + and user_id = #{userId} + </if> + <if test="status != 0"> + and status = #{status} + </if> + <if test="name != null and name != '' "> + and name like concat('%', #{name}, '%') + </if> + </where> + </select> + + <!--modify data by id--> + <update id="updateTaskGroupResource"> + update t_ds_task_group + set use_size = use_size+1 + where id = #{id} and use_size < group_size and + (select count(1) FROM t_ds_task_group_queue where id = #{queueId} and status = #{queueStatus} ) = 1 + </update> + + <!--modify data by id--> + <update id="releaseTaskGroupResource"> + update t_ds_task_group + set use_size = use_size-1 + where id = #{id} and use_size > 0 and + (select count(1) FROM t_ds_task_group_queue where id = #{queueId} and status = #{queueStatus} ) = 1 Review comment: the same ########## File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupQueueMapper.xml ########## @@ -0,0 +1,127 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<!DOCTYPE mapper PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN" "http://mybatis.org/dtd/mybatis-3-mapper.dtd";> +<mapper namespace="org.apache.dolphinscheduler.dao.mapper.TaskGroupQueueMapper"> + + <resultMap type="org.apache.dolphinscheduler.dao.entity.TaskGroupQueue" id="TaskGroupQueueMap"> + <result property="id" column="id" jdbcType="INTEGER"/> + <result property="taskId" column="task_id" jdbcType="INTEGER"/> + <result property="taskName" column="task_name" jdbcType="VARCHAR"/> + <result property="groupId" column="group_id" jdbcType="INTEGER"/> + <result property="processId" column="process_id" jdbcType="INTEGER"/> + <result property="priority" column="priority" jdbcType="INTEGER"/> + <result property="status" column="status" jdbcType="INTEGER"/> + <result property="forceStart" column="force_start" jdbcType="INTEGER"/> + <result property="inQueue" column="in_queue" jdbcType="INTEGER"/> + <result property="createTime" column="create_time" jdbcType="TIMESTAMP"/> + <result property="updateTime" column="update_time" jdbcType="TIMESTAMP"/> + </resultMap> + + <sql id = "baseSql"> + id, task_id, task_name, group_id, process_id, priority, status , force_start , in_queue, create_time, update_time + </sql> + + <select id="queryTaskGroupQueuePaging" resultType="org.apache.dolphinscheduler.dao.entity.TaskGroupQueue"> + select + <include refid="baseSql"> + </include> + from t_ds_task_group_queue + <where> + <if test="groupId != 0"> + and group_id = #{groupId} + </if> + <if test="processId != 0"> + and process_id = #{processId} + </if> + </where> + </select> + + <select id="queryByStatus" resultMap="TaskGroupQueueMap" resultType="map"> + select + <include refid="baseSql"> + </include> + from t_ds_task_group_queue + <where> + <if test="status != 0"> Review comment: Is `state=0` represents the initial state? it's a hidden logic for `queryByStatus` ########## File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupQueueMapper.xml ########## @@ -0,0 +1,127 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<!DOCTYPE mapper PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN" "http://mybatis.org/dtd/mybatis-3-mapper.dtd";> +<mapper namespace="org.apache.dolphinscheduler.dao.mapper.TaskGroupQueueMapper"> + + <resultMap type="org.apache.dolphinscheduler.dao.entity.TaskGroupQueue" id="TaskGroupQueueMap"> + <result property="id" column="id" jdbcType="INTEGER"/> + <result property="taskId" column="task_id" jdbcType="INTEGER"/> + <result property="taskName" column="task_name" jdbcType="VARCHAR"/> + <result property="groupId" column="group_id" jdbcType="INTEGER"/> + <result property="processId" column="process_id" jdbcType="INTEGER"/> + <result property="priority" column="priority" jdbcType="INTEGER"/> + <result property="status" column="status" jdbcType="INTEGER"/> + <result property="forceStart" column="force_start" jdbcType="INTEGER"/> + <result property="inQueue" column="in_queue" jdbcType="INTEGER"/> + <result property="createTime" column="create_time" jdbcType="TIMESTAMP"/> + <result property="updateTime" column="update_time" jdbcType="TIMESTAMP"/> + </resultMap> + + <sql id = "baseSql"> + id, task_id, task_name, group_id, process_id, priority, status , force_start , in_queue, create_time, update_time + </sql> + + <select id="queryTaskGroupQueuePaging" resultType="org.apache.dolphinscheduler.dao.entity.TaskGroupQueue"> + select + <include refid="baseSql"> + </include> + from t_ds_task_group_queue + <where> + <if test="groupId != 0"> + and group_id = #{groupId} + </if> + <if test="processId != 0"> + and process_id = #{processId} + </if> + </where> + </select> + + <select id="queryByStatus" resultMap="TaskGroupQueueMap" resultType="map"> + select + <include refid="baseSql"> + </include> + from t_ds_task_group_queue + <where> + <if test="status != 0"> + and status = #{status} + </if> + </where> + </select> + + <delete id="deleteByTaskId"> + delete from t_ds_task_group_queue + where task_id = #{taskId} + </delete> + + <update id="updateStatusByTaskId"> + update t_ds_task_group_queue + <set> + <if test="status != 0"> + status = #{status}, + </if> + </set> + where task_id = #{taskId} + </update> + + <update id="updateInQueue"> + update t_ds_task_group_queue + set in_queue = #{inQueue} + where id = #{id} + </update> + + <update id="updateForceStart"> + update t_ds_task_group_queue + set force_start = #{forceStart} + where task_id = #{taskId} + </update> + + <update id="updateInQueueLimit1"> + update t_ds_task_group_queue + set in_queue = #{newValue} + where group_id = #{groupId} and in_queue = #{oldValue} and status = #{status} order by priority desc limit 1 + </update> + + <update id="updateInQueueCAS"> + update t_ds_task_group_queue + set in_queue = #{newValue} + where id = #{id} and in_queue = #{oldValue} + </update> + + <select id="queryHighPriorityTasks" resultType="org.apache.dolphinscheduler.dao.entity.TaskGroupQueue"> + select + <include refid="baseSql" /> + from t_ds_task_group_queue + where group_id = #{groupId} and status = #{status} and priority > #{priority} + </select> + <select id="queryTheHighestPriorityTasks" resultType="org.apache.dolphinscheduler.dao.entity.TaskGroupQueue"> + select + <include refid="baseSql" /> + from t_ds_task_group_queue + where priority = (select max(priority) from t_ds_task_group_queue where group_id = #{groupId} Review comment: `select max(priority) from t_ds_task_group_queue where group_id = #{groupId} and status = #{status} and in_queue = #{inQueue} and force_start = #{forceStart}` Is it OK when this sub sql doesn't match anything? ########## File path: dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_mysql.sql ########## @@ -1017,3 +1020,40 @@ CREATE TABLE `t_ds_environment_worker_group_relation` ( PRIMARY KEY (`id`), UNIQUE KEY `environment_worker_group_unique` (`environment_code`,`worker_group`) ) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8; + +-- ---------------------------- +-- Table structure for t_ds_task_group_queue +-- ---------------------------- +DROP TABLE IF EXISTS `t_ds_task_group_queue`; +CREATE TABLE `t_ds_task_group_queue` ( + `id` int(11) NOT NULL AUTO_INCREMENT COMMENT'key', + `task_id` int(11) DEFAULT NULL COMMENT 'taskintanceid', + `task_name` varchar(100) DEFAULT NULL COMMENT 'TaskInstance name', + `group_id` int(11) DEFAULT NULL COMMENT 'taskGroup id', + `process_id` int(11) DEFAULT NULL COMMENT 'processInstace id', + `priority` int(8) DEFAULT '0' COMMENT 'priority', + `status` tinyint(4) DEFAULT '-1' COMMENT '-1: waiting 1: running 2: finished', + `force_start` tinyint(4) DEFAULT '0' COMMENT 'is force start 0 NO ,1 YES', + `in_queue` tinyint(4) DEFAULT '0' COMMENT 'ready to get the queue by other task finish 0 NO ,1 YES', + `create_time` timestamp NULL DEFAULT CURRENT_TIMESTAMP, + `update_time` timestamp NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + PRIMARY KEY( `id` ) Review comment: Does it need to add any index? ########## File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.xml ########## @@ -0,0 +1,87 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<!DOCTYPE mapper PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN" "http://mybatis.org/dtd/mybatis-3-mapper.dtd";> +<mapper namespace="org.apache.dolphinscheduler.dao.mapper.TaskGroupMapper"> + + <resultMap type="org.apache.dolphinscheduler.dao.entity.TaskGroup" id="TaskGroupMap"> + <result property="id" column="id" jdbcType="INTEGER"/> + <result property="name" column="name" jdbcType="VARCHAR"/> + <result property="description" column="description" jdbcType="VARCHAR"/> + <result property="groupSize" column="group_size" jdbcType="INTEGER"/> + <result property="useSize" column="use_size" jdbcType="INTEGER"/> + <result property="userId" column="user_id" jdbcType="INTEGER"/> + <result property="status" column="status" jdbcType="VARCHAR"/> + <result property="createTime" column="create_time" jdbcType="TIMESTAMP"/> + <result property="updateTime" column="update_time" jdbcType="TIMESTAMP"/> + </resultMap> + + <sql id = "baseSql"> + id,name,description,group_size,use_size,create_time,update_time + </sql> + + <select id="queryTaskGroupPaging" resultType="org.apache.dolphinscheduler.dao.entity.TaskGroup"> + select + <include refid="baseSql"> + </include> + from t_ds_task_group + <where> + <if test="userId != 0"> + and user_id = #{userId} + </if> + <if test="status != 0"> + and status = #{status} + </if> + <if test="name != null and name != '' "> + and name like concat('%', #{name}, '%') + </if> + </where> + </select> + + <!--modify data by id--> + <update id="updateTaskGroupResource"> + update t_ds_task_group + set use_size = use_size+1 + where id = #{id} and use_size < group_size and + (select count(1) FROM t_ds_task_group_queue where id = #{queueId} and status = #{queueStatus} ) = 1 Review comment: Why not split this sql and valid it before update? It can keep the sql logic more clearly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
