You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/06/08 03:27:16 UTC
[GitHub] [dolphinscheduler] SbloodyS opened a new pull request, #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
SbloodyS opened a new pull request, #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373
<!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
## Purpose of the pull request
close #6758
## Brief change log
Add task resource usage limit base on ```systemd``` and ```cgroup```.
For more information you can see ```https://www.freedesktop.org/software/systemd/man/systemd-run.html``` and ```https://www.freedesktop.org/software/systemd/man/systemd.resource-control.html#```.
Due to CGroup only exists in Linux, the current resource limit only supports Linux. I think this is enough for now since big data scheduling tasks only run in Linux.
-->
## Verify this pull request
I've test successfully in ```Ubuntu 16.04``` and ```Ubuntu 18.04``` and ```Centos7```.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891954336
##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/resources/docker/file-manage/common.properties:
##########
@@ -85,3 +85,6 @@ aws.access.key.id=accessKey123
aws.secret.access.key=secretKey123
aws.region=us-east-1
aws.endpoint=http://s3:9000
+
+# Task resource limit state
+task.resource.limit.state=false
Review Comment:
and in this case, we should add some doc for users about how to enable the CPU and mem config
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891983337
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
>how about using single column as string and store and parse json
Do you mean the `task_params`?
I think if the parameter is generic and matches all tasks, it can add a new column.
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
>how about using single column as string and store and parse json
Do you mean the `task_params`?
I think if the parameter is generic and matches all tasks, it can add a new column.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r892035841
##########
docs/docs/en/guide/task/datax.md:
##########
@@ -19,6 +19,8 @@ DataX task type for executing DataX programs. For DataX nodes, the worker will e
- **Environment Name**: Configure the environment name in which to run the script.
- **Number of failed retry attempts**: The number of times the task failed to be resubmitted.
- **Failed retry interval**: The time, in cents, interval for resubmitting the task after a failed task.
+- **Cpu quota**: Assign the specified CPU time quota to the task executed. Takes a percentage value. Default -1 means unlimited. For example, the full CPU load of one core is 100%,and that of 16 cores is 1600%. This function is controlled by [task.resource.limit.state](https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/architecture/configuration.html)
Review Comment:
change to ```<version>``` may cause dead link check failed....Is there a workaround?
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891942836
##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/resources/docker/file-manage/common.properties:
##########
@@ -85,3 +85,6 @@ aws.access.key.id=accessKey123
aws.secret.access.key=secretKey123
aws.region=us-east-1
aws.endpoint=http://s3:9000
+
+# Task resource limit state
+task.resource.limit.state=false
Review Comment:
> we set false by default, and it will not available when user set any number of CPU and mem in web UI, should we set `true` as default?
I had a discussion with @kezhenxu94 in this problemes. Because our current E2E test is based on docker, ```systemd``` and ```CGroup``` are not supported by default in the docker container. In the future, I will try to enable non docker mode tests in E2E and then improve the tests.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#issuecomment-1149426940
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10373)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_coverage&view=list) [29.3% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie merged pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie merged PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891940928
##########
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/entity/TaskInstance.java:
##########
@@ -753,4 +763,20 @@ public int getTaskGroupPriority() {
public void setTaskGroupPriority(int taskGroupPriority) {
this.taskGroupPriority = taskGroupPriority;
}
+
+ public Integer getCpuQuota() {
+ return cpuQuota;
+ }
+
+ public void setCpuQuota(Integer cpuQuota) {
+ this.cpuQuota = cpuQuota;
Review Comment:
I'll fix it.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891956831
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
We just split json in version 2.0+. I think we should not add more json columns on it...
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r893039335
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/OSUtils.java:
##########
@@ -68,6 +69,15 @@ public static boolean isWindows() {
return getOSName().startsWith("Windows");
}
+ /**
+ * whether is linux
+ *
+ * @return true if linux
+ */
+ public static boolean isLinux() {
+ return SystemUtils.IS_OS_LINUX;
+ }
Review Comment:
Sure. I'll do this within tomorrow.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891983337
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
> how about using single column as string and store and parse json
Do you mean the `task_params`?
I think if the parameter is generic and matches all tasks, it can add a new column.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r892002958
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
> > I think if the parameter is generic and matches all tasks, it can add a new column.
>
> But it is not, it only for some of task types which may download resource file form resource center. and user have to use those resource file. for example users can use or can not use shell with resource files.
ignore this comment, I finally get that we want to limit the linux resource in task, in this case I think we could add new column for it 🤣
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891994294
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
> I think if the parameter is generic and matches all tasks, it can add a new column.
But it is not, it only for some of task types which may download resource file form resource center. and user have to use those resource file. for example users can use or can not use shell with resource files.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#issuecomment-1149637139
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10373)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_coverage&view=list) [32.1% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_duplicated_lines_density&view=list) [9.5% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891896369
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/OSUtils.java:
##########
@@ -68,6 +68,15 @@ public static boolean isWindows() {
return getOSName().startsWith("Windows");
}
+ /**
+ * whether is linux
+ *
+ * @return true if linux
+ */
+ public static boolean isLinux() {
+ return !isWindows() && !isMacOS();
+ }
Review Comment:
It’s better to directly use `SystemUtils.IS_OS_LINUX`, this may need to add `commons.lang3` in task api module.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] Amy0104 commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
Amy0104 commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891924391
##########
dolphinscheduler-ui/src/locales/en_US/project.ts:
##########
@@ -308,6 +308,8 @@ export default {
task_group_name: 'Task group name',
task_group_queue_priority: 'Priority',
number_of_failed_retries: 'Number of failed retries',
+ cpu_quota: 'Cpu quota',
Review Comment:
It seems to be better that the cpu be capitalized.
##########
dolphinscheduler-ui/src/views/projects/task/components/node/format-data.ts:
##########
@@ -409,7 +409,9 @@ export function formatParams(data: INodeData): {
timeout: data.timeoutFlag ? data.timeout : 0,
timeoutFlag: data.timeoutFlag ? 'OPEN' : 'CLOSE',
timeoutNotifyStrategy: data.timeoutFlag ? timeoutNotifyStrategy : '',
- workerGroup: data.workerGroup
+ workerGroup: data.workerGroup,
+ cpuQuota: data.cpuQuota,
+ memoryMax: data.memoryMax
Review Comment:
It is better to set a default value for the cupQuota and the memoryMax when other tasks don't need the two parameters.
##########
dolphinscheduler-ui/src/locales/zh_CN/project.ts:
##########
@@ -308,6 +308,8 @@ export default {
task_group_name: '任务组名称',
task_group_queue_priority: '组内优先级',
number_of_failed_retries: '失败重试次数',
+ cpu_quota: 'Cpu配额',
Review Comment:
Same as above.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891959408
##########
dolphinscheduler-ui/src/views/projects/task/components/node/format-data.ts:
##########
@@ -409,7 +409,9 @@ export function formatParams(data: INodeData): {
timeout: data.timeoutFlag ? data.timeout : 0,
timeoutFlag: data.timeoutFlag ? 'OPEN' : 'CLOSE',
timeoutNotifyStrategy: data.timeoutFlag ? timeoutNotifyStrategy : '',
- workerGroup: data.workerGroup
+ workerGroup: data.workerGroup,
+ cpuQuota: data.cpuQuota,
+ memoryMax: data.memoryMax
Review Comment:
Done.
##########
dolphinscheduler-ui/src/locales/zh_CN/project.ts:
##########
@@ -308,6 +308,8 @@ export default {
task_group_name: '任务组名称',
task_group_queue_priority: '组内优先级',
number_of_failed_retries: '失败重试次数',
+ cpu_quota: 'Cpu配额',
Review Comment:
Done.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r892002930
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteThread.java:
##########
@@ -1307,6 +1311,7 @@ private void submitPostNode(String parentNodeCode) {
}
TaskInstance task = createTaskInstance(processInstance, taskNodeObject);
taskInstances.add(task);
+ logger.info("task {} add to stand by list, cpuQuota: {}", task.getName(), task.getCpuQuota());
Review Comment:
Done.
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractCommandExecutor.java:
##########
@@ -142,6 +148,34 @@ private void buildProcess(String commandFile) throws IOException {
printCommand(command);
}
+ private void generateCgroupCommand(List<String> command) {
Review Comment:
Done.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r892004426
##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/resources/docker/file-manage/common.properties:
##########
@@ -85,3 +85,6 @@ aws.access.key.id=accessKey123
aws.secret.access.key=secretKey123
aws.region=us-east-1
aws.endpoint=http://s3:9000
+
+# Task resource limit state
+task.resource.limit.state=false
Review Comment:
> how about mention in each task "Cpu quota" and "Memery usage" and ref to docs/docs/en/architecture/configuration.md
Done. Please take a look.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r892025650
##########
docs/docs/en/guide/task/datax.md:
##########
@@ -19,6 +19,8 @@ DataX task type for executing DataX programs. For DataX nodes, the worker will e
- **Environment Name**: Configure the environment name in which to run the script.
- **Number of failed retry attempts**: The number of times the task failed to be resubmitted.
- **Failed retry interval**: The time, in cents, interval for resubmitting the task after a failed task.
+- **Cpu quota**: Assign the specified CPU time quota to the task executed. Takes a percentage value. Default -1 means unlimited. For example, the full CPU load of one core is 100%,and that of 16 cores is 1600%. This function is controlled by [task.resource.limit.state](https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/architecture/configuration.html)
Review Comment:
we should avoid using `lastest` in our document, and should use place hold `<version>` then replace them in our release process. for more detail see https://dolphinscheduler.apache.org/en-us/community/release-prepare.html section "Update Version"
```suggestion
- **Cpu quota**: Assign the specified CPU time quota to the task executed. Takes a percentage value. Default -1 means unlimited. For example, the full CPU load of one core is 100%,and that of 16 cores is 1600%. This function is controlled by [task.resource.limit.state](https://dolphinscheduler.apache.org/en-us/docs/<version>/user_doc/architecture/configuration.html)
```
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r892046738
##########
docs/docs/en/guide/task/datax.md:
##########
@@ -19,6 +19,8 @@ DataX task type for executing DataX programs. For DataX nodes, the worker will e
- **Environment Name**: Configure the environment name in which to run the script.
- **Number of failed retry attempts**: The number of times the task failed to be resubmitted.
- **Failed retry interval**: The time, in cents, interval for resubmitting the task after a failed task.
+- **Cpu quota**: Assign the specified CPU time quota to the task executed. Takes a percentage value. Default -1 means unlimited. For example, the full CPU load of one core is 100%,and that of 16 cores is 1600%. This function is controlled by [task.resource.limit.state](https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/architecture/configuration.html)
Review Comment:
try using related path `[task.resource.limit.state](../../architecture/configuration.md)`
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891955222
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
how about using single column as string and store and parse json
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891992734
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteThread.java:
##########
@@ -1307,6 +1311,7 @@ private void submitPostNode(String parentNodeCode) {
}
TaskInstance task = createTaskInstance(processInstance, taskNodeObject);
taskInstances.add(task);
+ logger.info("task {} add to stand by list, cpuQuota: {}", task.getName(), task.getCpuQuota());
Review Comment:
My mistake...I'll remove it.
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractCommandExecutor.java:
##########
@@ -142,6 +148,34 @@ private void buildProcess(String commandFile) throws IOException {
printCommand(command);
}
+ private void generateCgroupCommand(List<String> command) {
Review Comment:
Sure.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891968377
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
I am not sure about that, do you have time to take a look this discuss @caishunfeng @lenboo
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891967983
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
but currently our task param still in json now
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#issuecomment-1149463938
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10373)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_coverage&view=list) [30.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891989936
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteThread.java:
##########
@@ -1307,6 +1311,7 @@ private void submitPostNode(String parentNodeCode) {
}
TaskInstance task = createTaskInstance(processInstance, taskNodeObject);
taskInstances.add(task);
+ logger.info("task {} add to stand by list, cpuQuota: {}", task.getName(), task.getCpuQuota());
Review Comment:
remove if not use.
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractCommandExecutor.java:
##########
@@ -142,6 +148,34 @@ private void buildProcess(String commandFile) throws IOException {
printCommand(command);
}
+ private void generateCgroupCommand(List<String> command) {
Review Comment:
can we add some example comment here?
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#issuecomment-1149574901
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10373)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_coverage&view=list) [32.1% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_duplicated_lines_density&view=list) [9.5% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r892061074
##########
docs/docs/en/guide/task/datax.md:
##########
@@ -19,6 +19,8 @@ DataX task type for executing DataX programs. For DataX nodes, the worker will e
- **Environment Name**: Configure the environment name in which to run the script.
- **Number of failed retry attempts**: The number of times the task failed to be resubmitted.
- **Failed retry interval**: The time, in cents, interval for resubmitting the task after a failed task.
+- **Cpu quota**: Assign the specified CPU time quota to the task executed. Takes a percentage value. Default -1 means unlimited. For example, the full CPU load of one core is 100%,and that of 16 cores is 1600%. This function is controlled by [task.resource.limit.state](https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/architecture/configuration.html)
Review Comment:
It works. Thanks.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r892046738
##########
docs/docs/en/guide/task/datax.md:
##########
@@ -19,6 +19,8 @@ DataX task type for executing DataX programs. For DataX nodes, the worker will e
- **Environment Name**: Configure the environment name in which to run the script.
- **Number of failed retry attempts**: The number of times the task failed to be resubmitted.
- **Failed retry interval**: The time, in cents, interval for resubmitting the task after a failed task.
+- **Cpu quota**: Assign the specified CPU time quota to the task executed. Takes a percentage value. Default -1 means unlimited. For example, the full CPU load of one core is 100%,and that of 16 cores is 1600%. This function is controlled by [task.resource.limit.state](https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/architecture/configuration.html)
Review Comment:
try using related path [task.resource.limit.state](../../architecture/configuration.md)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r892042060
##########
docs/docs/en/guide/task/datax.md:
##########
@@ -19,6 +19,8 @@ DataX task type for executing DataX programs. For DataX nodes, the worker will e
- **Environment Name**: Configure the environment name in which to run the script.
- **Number of failed retry attempts**: The number of times the task failed to be resubmitted.
- **Failed retry interval**: The time, in cents, interval for resubmitting the task after a failed task.
+- **Cpu quota**: Assign the specified CPU time quota to the task executed. Takes a percentage value. Default -1 means unlimited. For example, the full CPU load of one core is 100%,and that of 16 cores is 1600%. This function is controlled by [task.resource.limit.state](https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/architecture/configuration.html)
Review Comment:
🤣
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#issuecomment-1149525847
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10373)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10373&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_coverage&view=list) [31.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_duplicated_lines_density&view=list) [9.6% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10373&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891942836
##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/resources/docker/file-manage/common.properties:
##########
@@ -85,3 +85,6 @@ aws.access.key.id=accessKey123
aws.secret.access.key=secretKey123
aws.region=us-east-1
aws.endpoint=http://s3:9000
+
+# Task resource limit state
+task.resource.limit.state=false
Review Comment:
> we set false by default, and it will not available when user set any number of CPU and mem in web UI, should we set `true` as default?
I had a discussion with @kezhenxu94 in this problems. Because our current E2E test is based on docker, ```systemd``` and ```CGroup``` are not supported by default in the docker container. In the future, I will try to enable non docker mode tests in E2E and then improve the tests.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891940437
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
I've thought about it, too. But I can't find a field to put these two contents without affecting other functions....
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891912592
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/OSUtils.java:
##########
@@ -68,6 +68,15 @@ public static boolean isWindows() {
return getOSName().startsWith("Windows");
}
+ /**
+ * whether is linux
+ *
+ * @return true if linux
+ */
+ public static boolean isLinux() {
+ return !isWindows() && !isMacOS();
+ }
Review Comment:
Done.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891994294
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
> I think if the parameter is generic and matches all tasks, it can add a new column.
But it is not, it only for some of task types which may download resource file form resource center. and user have to use those resource file. for example users could use shell without resource file alone.
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
> I think if the parameter is generic and matches all tasks, it can add a new column.
But it is not, it only for some of task types which may download resource file form resource center. and user have to use those resource file. for example users could use shell without resource file alone.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#issuecomment-1149807243
merging……
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891917581
##########
docs/docs/en/guide/task/datax.md:
##########
@@ -19,6 +19,8 @@ DataX task type for executing DataX programs. For DataX nodes, the worker will e
- **Environment Name**: Configure the environment name in which to run the script.
- **Number of failed retry attempts**: The number of times the task failed to be resubmitted.
- **Failed retry interval**: The time, in cents, interval for resubmitting the task after a failed task.
+- **Cpu quota**: Assign the specified CPU time quota to the task executed. Takes a percentage value. Default -1 means unlimited. For example, the full CPU load of one core is 100%,and that of 16 cores is 1600%.
Review Comment:
Thanks for given an example about how to config it
##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/resources/docker/file-manage/common.properties:
##########
@@ -85,3 +85,6 @@ aws.access.key.id=accessKey123
aws.secret.access.key=secretKey123
aws.region=us-east-1
aws.endpoint=http://s3:9000
+
+# Task resource limit state
+task.resource.limit.state=false
Review Comment:
we set false by default, and it will not available when user set any number of CPU and mem in web UI, should we set `true` as default?
##########
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/entity/TaskInstance.java:
##########
@@ -753,4 +763,20 @@ public int getTaskGroupPriority() {
public void setTaskGroupPriority(int taskGroupPriority) {
this.taskGroupPriority = taskGroupPriority;
}
+
+ public Integer getCpuQuota() {
+ return cpuQuota;
+ }
+
+ public void setCpuQuota(Integer cpuQuota) {
+ this.cpuQuota = cpuQuota;
Review Comment:
I have a question here, should we handle the `null` value in all set CPU and mem place to keep consistency. I know taskdefinitionlog and task instance come from taskdefintion, but I think keep consistency is not bad for all cases. But I am not have strong opinion here
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
I think we should discuss about thoes column adding. Out table column is too many IMO, and this property is only use by some of the tasks not all of them, do we really need to add them into separate columns? should we add some column name `extra` or some name else, and put those two columns and others extra column only for some of the tasks in the further to keep our column as less column as possible?
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891926168
##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/resources/docker/file-manage/common.properties:
##########
@@ -85,3 +85,6 @@ aws.access.key.id=accessKey123
aws.secret.access.key=secretKey123
aws.region=us-east-1
aws.endpoint=http://s3:9000
+
+# Task resource limit state
+task.resource.limit.state=false
Review Comment:
BTW, what ever we use, I think we should add this config in our document to notice users
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891942952
##########
dolphinscheduler-ui/src/locales/en_US/project.ts:
##########
@@ -308,6 +308,8 @@ export default {
task_group_name: 'Task group name',
task_group_queue_priority: 'Priority',
number_of_failed_retries: 'Number of failed retries',
+ cpu_quota: 'Cpu quota',
Review Comment:
I'll fix it.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r893038864
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/OSUtils.java:
##########
@@ -68,6 +69,15 @@ public static boolean isWindows() {
return getOSName().startsWith("Windows");
}
+ /**
+ * whether is linux
+ *
+ * @return true if linux
+ */
+ public static boolean isLinux() {
+ return SystemUtils.IS_OS_LINUX;
+ }
Review Comment:
Can you replace (in another PR) all these `isXxx` with `common.lang3` and inline these methods? Instead of wrapping them again in a method
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#issuecomment-1149423131
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10373?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10373](https://codecov.io/gh/apache/dolphinscheduler/pull/10373?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cc7d314) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/36bba3fcdff58176a27b71155db6539244496106?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (36bba3f) will **decrease** coverage by `0.01%`.
> The diff coverage is `32.78%`.
```diff
@@ Coverage Diff @@
## dev #10373 +/- ##
============================================
- Coverage 40.57% 40.56% -0.02%
- Complexity 4741 4752 +11
============================================
Files 869 870 +1
Lines 35286 35344 +58
Branches 3935 3940 +5
============================================
+ Hits 14318 14336 +18
- Misses 19538 19575 +37
- Partials 1430 1433 +3
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/10373?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/dolphinscheduler/common/model/TaskNode.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL21vZGVsL1Rhc2tOb2RlLmphdmE=) | `24.03% <0.00%> (-1.18%)` | :arrow_down: |
| [...dolphinscheduler/dao/entity/TaskDefinitionLog.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYW8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvZGFvL2VudGl0eS9UYXNrRGVmaW5pdGlvbkxvZy5qYXZh) | `19.04% <0.00%> (-0.96%)` | :arrow_down: |
| [...er/server/master/runner/WorkflowExecuteThread.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvV29ya2Zsb3dFeGVjdXRlVGhyZWFkLmphdmE=) | `7.93% <0.00%> (-0.03%)` | :arrow_down: |
| [...duler/plugin/task/api/AbstractCommandExecutor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL2FwaS9BYnN0cmFjdENvbW1hbmRFeGVjdXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...sk/api/utils/AbstractCommandExecutorConstants.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL2FwaS91dGlscy9BYnN0cmFjdENvbW1hbmRFeGVjdXRvckNvbnN0YW50cy5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...olphinscheduler/plugin/task/api/utils/OSUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL2FwaS91dGlscy9PU1V0aWxzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...he/dolphinscheduler/dao/entity/TaskDefinition.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYW8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvZGFvL2VudGl0eS9UYXNrRGVmaW5pdGlvbi5qYXZh) | `71.65% <50.00%> (-1.46%)` | :arrow_down: |
| [...ache/dolphinscheduler/dao/entity/TaskInstance.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYW8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvZGFvL2VudGl0eS9UYXNrSW5zdGFuY2UuamF2YQ==) | `65.69% <100.00%> (+1.23%)` | :arrow_up: |
| [...er/server/builder/TaskExecutionContextBuilder.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL2J1aWxkZXIvVGFza0V4ZWN1dGlvbkNvbnRleHRCdWlsZGVyLmphdmE=) | `70.21% <100.00%> (+1.32%)` | :arrow_up: |
| [...nscheduler/service/process/ProcessServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvcHJvY2Vzcy9Qcm9jZXNzU2VydmljZUltcGwuamF2YQ==) | `30.73% <100.00%> (+0.11%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/dolphinscheduler/pull/10373/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10373?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10373?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [36bba3f...cc7d314](https://codecov.io/gh/apache/dolphinscheduler/pull/10373?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891955747
##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/resources/docker/file-manage/common.properties:
##########
@@ -85,3 +85,6 @@ aws.access.key.id=accessKey123
aws.secret.access.key=secretKey123
aws.region=us-east-1
aws.endpoint=http://s3:9000
+
+# Task resource limit state
+task.resource.limit.state=false
Review Comment:
> and in this case, we should add some doc for users about how to enable the CPU and mem config
I was wondering which place should we put this to the docs?
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891967983
##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql:
##########
@@ -486,6 +486,8 @@ CREATE TABLE t_ds_task_definition
delay_time int(11) DEFAULT '0',
task_group_id int(11) DEFAULT NULL,
task_group_priority tinyint(4) DEFAULT '0',
+ cpu_quota int(11) DEFAULT '-1' NOT NULL,
+ memory_max int(11) DEFAULT '-1' NOT NULL,
Review Comment:
but currently, our task param is still in JSON now. and I think this is only some optional config for some of the tasks, it is OK IMO
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891973712
##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/resources/docker/file-manage/common.properties:
##########
@@ -85,3 +85,6 @@ aws.access.key.id=accessKey123
aws.secret.access.key=secretKey123
aws.region=us-east-1
aws.endpoint=http://s3:9000
+
+# Task resource limit state
+task.resource.limit.state=false
Review Comment:
> how about mention in each task "Cpu quota" and "Memery usage" and ref to docs/docs/en/architecture/configuration.md
I'll add it.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10373: [Feature-6758][Task] Add limit resource usage for tasks base on cgroup
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10373:
URL: https://github.com/apache/dolphinscheduler/pull/10373#discussion_r891972658
##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/resources/docker/file-manage/common.properties:
##########
@@ -85,3 +85,6 @@ aws.access.key.id=accessKey123
aws.secret.access.key=secretKey123
aws.region=us-east-1
aws.endpoint=http://s3:9000
+
+# Task resource limit state
+task.resource.limit.state=false
Review Comment:
how about mention in each task "Cpu quota" and "Memery usage" and ref to docs/docs/en/architecture/configuration.md
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org