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/09/28 13:04:53 UTC
[GitHub] [dolphinscheduler] Radeity opened a new pull request, #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Radeity opened a new pull request, #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197
<!--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
- Import `aop` way as an alternative way to collect yarn job's applicationId.
- Also, this PR closes https://github.com/apache/dolphinscheduler/issues/11262
## Brief change log
- Create new module `dolphinscheduler-aop` for `aop` code.
- Add new environment configuration for each type of yarn tasks to support `aop`, as follow:
```shell
export HADOOP_CLASSPATH=${DOLPHINSCHEDULER_HOME}/tools/libs/aop/*:$HADOOP_CLASSPATH
export SPARK_DIST_CLASSPATH=${DOLPHINSCHEDULER_HOME}/tools/libs/aop/*:$SPARK_DIST_CLASS_PATH
export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
```
- Add user properties for user to decide how to collect applicationId, as follow:
```shell
# way to collect applicationId: log(original regex match), aop
appId.collect: log
# appIds info log path
appId.file.path = appInfo.log
```
## Verify this pull request
- Service logic is verified by unit test.
- Effectiveness of `aop` for supported types of yarn job are simply tested on my local cluster.
--
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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983187618
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -75,14 +75,14 @@ public static void checkTenantExist(WorkerConfig workerConfig, TaskExecutionCont
public static void createProcessLocalPathIfAbsent(TaskExecutionContext taskExecutionContext) throws TaskException {
try {
// local execute path
- String execLocalPath = FileUtils.getProcessExecDir(
- taskExecutionContext.getProjectCode(),
- taskExecutionContext.getProcessDefineCode(),
- taskExecutionContext.getProcessDefineVersion(),
- taskExecutionContext.getProcessInstanceId(),
- taskExecutionContext.getTaskInstanceId());
- taskExecutionContext.setExecutePath(execLocalPath);
- FileUtils.createWorkDirIfAbsent(execLocalPath);
+ // String execLocalPath = FileUtils.getProcessExecDir(
+ // taskExecutionContext.getProjectCode(),
+ // taskExecutionContext.getProcessDefineCode(),
+ // taskExecutionContext.getProcessDefineVersion(),
+ // taskExecutionContext.getProcessInstanceId(),
+ // taskExecutionContext.getTaskInstanbu iceId());
+ // taskExecutionContext.setExecutePath(execLocalPath);
+ FileUtils.createWorkDirIfAbsent(taskExecutionContext.getExecutePath());
Review Comment:
>
I put it in `TaskExecutionContextBuilder` when setting some task related attribution, together with setting of `appInfoPath`.
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1262176938
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [11 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [6.1% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1007685050
##########
script/env/dolphinscheduler_env.sh:
##########
@@ -33,3 +33,10 @@ export SEATUNNEL_HOME=${SEATUNNEL_HOME:-/opt/soft/seatunnel}
export CHUNJUN_HOME=${CHUNJUN_HOME:-/opt/soft/chunjun}
export PATH=$HADOOP_HOME/bin:$SPARK_HOME/bin:$PYTHON_HOME/bin:$JAVA_HOME/bin:$HIVE_HOME/bin:$FLINK_HOME/bin:$DATAX_HOME/bin:$SEATUNNEL_HOME/bin:$CHUNJUN_HOME/bin:$PATH
+
+# applicationId auto collection related configuration
+export HADOOP_CLASSPATH=`hadoop classpath`:${DOLPHINSCHEDULER_HOME}/tools/libs/*
+export SPARK_DIST_CLASSPATH=$HADOOP_CLASSPATH:$SPARK_DIST_CLASS_PATH
+export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
+export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
+export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
Review Comment:
@caishunfeng Please have a look at the latest changes, include most of your suggestions
--
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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006532774
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/builder/TaskExecutionContextBuilder.java:
##########
@@ -126,6 +127,24 @@ public TaskExecutionContextBuilder buildProcessDefinitionRelatedInfo(ProcessDefi
return this;
}
+ /**
+ * build execPath related info
+ *
+ * @return TaskExecutionContextBuilder
+ */
+ public TaskExecutionContextBuilder buildExecPathRelatedInfo() {
Review Comment:
> > I think it should setExecutePath and appInfoPath in worker before task running. cc @ruanwenjun
>
> Yes, it should be moved to worker and should be set before yarn task running, I am not sure if other task will need this, you can simply put this before all task running.
>
> @Radeity In fact, the master doesn't know execPath, it is defined in worker, different worker may have different path, master doesn't know the execpath
Oops, Thanks for reminding! I have ignored that different workers may have different `DATA_BASEDIR`. I'll change this part of code, but, i'll still keep `appInfoPath`, is that okay?
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/builder/TaskExecutionContextBuilder.java:
##########
@@ -126,6 +127,24 @@ public TaskExecutionContextBuilder buildProcessDefinitionRelatedInfo(ProcessDefi
return this;
}
+ /**
+ * build execPath related info
+ *
+ * @return TaskExecutionContextBuilder
+ */
+ public TaskExecutionContextBuilder buildExecPathRelatedInfo() {
Review Comment:
> > I think it should setExecutePath and appInfoPath in worker before task running. cc @ruanwenjun
>
> Yes, it should be moved to worker and should be set before yarn task running, I am not sure if other task will need this, you can simply put this before all task running.
>
> @Radeity In fact, the master doesn't know execPath, it is defined in worker, different worker may have different path, master doesn't know the execpath
Oops, thanks for reminding! I have ignored that different workers may have different `DATA_BASEDIR`. I'll change this part of code, but, i'll still keep `appInfoPath`, is that okay?
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1294673415
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [23.2% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] sonarcloud[bot] commented on pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1293745928
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [24.2% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983210893
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * flag to indicate whether print debug logs
+ */
+ private static final String PARA_NAME_ASPECTJ_DEBUG = "PARA_NAME_ASPECTJ_DEBUG";
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = System.getProperty("user.dir") + "/appInfo.log";
+ debug = Boolean.parseBoolean(System.getenv(PARA_NAME_ASPECTJ_DEBUG));
Review Comment:
> should we put this variable `PARA_NAME_ASPECTJ_DEBUG` into common.properties?
AFAIK, `PARA_NAME_ASPECTJ_DEBUG` can not be read if put in common.properties, because task and aop code runs in another JVM and different JVM can not share properties in normal way.
--
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] github-code-scanning[bot] commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983200257
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * flag to indicate whether print debug logs
+ */
+ private static final String PARA_NAME_ASPECTJ_DEBUG = "PARA_NAME_ASPECTJ_DEBUG";
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = System.getProperty("user.dir") + "/appInfo.log";
+ debug = Boolean.parseBoolean(System.getenv(PARA_NAME_ASPECTJ_DEBUG));
+ }
+
+ /**
+ * Trigger submitApplication when invoking YarnClientImpl.submitApplication
+ *
+ * @param appContext application context when invoking YarnClientImpl.submitApplication
+ * @param submittedAppId the submitted application id returned by YarnClientImpl.submitApplication
+ * @throws Throwable exceptions
+ */
+ @AfterReturning(pointcut = "execution(ApplicationId org.apache.hadoop.yarn.client.api.impl.YarnClientImpl." +
+ "submitApplication(ApplicationSubmissionContext)) && args(appContext)",
+ returning = "submittedAppId", argNames = "appContext,submittedAppId")
+ public void registerApplicationInfo(ApplicationSubmissionContext appContext, ApplicationId submittedAppId) {
+ if (appInfoFilePath != null) {
+ try {
+ Files.write(Paths.get(appInfoFilePath),
+ Collections.singletonList(submittedAppId.toString()),
+ StandardOpenOption.CREATE,
+ StandardOpenOption.WRITE,
+ StandardOpenOption.APPEND);
+ } catch (IOException ioException) {
+ System.out.println(
+ "YarnClientAspect[registerAppInfo]: can't output current application information, because "
+ + ioException.getMessage());
+ }
+ }
+ if (debug) {
+ System.out.println("YarnClientAspect[submitApplication]: current application context " + appContext);
+ System.out.println("YarnClientAspect[submitApplication]: submitted application id " + submittedAppId);
+ System.out.println(
+ "YarnClientAspect[submitApplication]: current application report " + currentApplicationReport);
+ }
+ }
+
+ /**
+ * Trigger getAppReport only when invoking getApplicationReport within submitApplication
+ * This method will invoke many times, however, the last ApplicationReport instance assigned to currentApplicationReport
+ *
+ * @param appReport current application report when invoking getApplicationReport within submitApplication
+ * @param appId current application id, which is the parameter of getApplicationReport
+ * @throws Throwable exceptions
+ */
+ @AfterReturning(pointcut = "cflow(execution(ApplicationId org.apache.hadoop.yarn.client.api.impl.YarnClientImpl.submitApplication(ApplicationSubmissionContext))) "
+ +
+ "&& !within(CfowAspect) && execution(ApplicationReport org.apache.hadoop.yarn.client.api.impl.YarnClientImpl.getApplicationReport(ApplicationId)) && args(appId)", returning = "appReport", argNames = "appReport,appId")
+ public void registerApplicationReport(ApplicationReport appReport, ApplicationId appId) {
Review Comment:
## Useless parameter
The parameter appId is unused.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1580)
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1262180059
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [11 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [6.1% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] sonarcloud[bot] commented on pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1289870067
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [21.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006420795
##########
dolphinscheduler-aop/pom.xml:
##########
@@ -0,0 +1,91 @@
+<?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.
+ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.dolphinscheduler</groupId>
+ <artifactId>dolphinscheduler</artifactId>
+ <version>dev-SNAPSHOT</version>
+ </parent>
+ <artifactId>dolphinscheduler-aop</artifactId>
+ <packaging>jar</packaging>
+ <name>${project.artifactId}</name>
+ <description>aop 4 YarnClient to get application id when submitting jars using 'yarn jar mainClass args'</description>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>1.8</maven.compiler.source>
+ <maven.compiler.target>1.8</maven.compiler.target>
+ <aspectj.version>1.9.7</aspectj.version>
+ <hadoop.version>3.2.4</hadoop.version>
Review Comment:
It makes sense, 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] Radeity commented on pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1288434765
@ruanwenjun @SbloodyS @EricGao888
Would you like to help review the code? I'll handle the conflict later today.
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1007531303
##########
script/env/dolphinscheduler_env.sh:
##########
@@ -33,3 +33,10 @@ export SEATUNNEL_HOME=${SEATUNNEL_HOME:-/opt/soft/seatunnel}
export CHUNJUN_HOME=${CHUNJUN_HOME:-/opt/soft/chunjun}
export PATH=$HADOOP_HOME/bin:$SPARK_HOME/bin:$PYTHON_HOME/bin:$JAVA_HOME/bin:$HIVE_HOME/bin:$FLINK_HOME/bin:$DATAX_HOME/bin:$SEATUNNEL_HOME/bin:$CHUNJUN_HOME/bin:$PATH
+
+# applicationId auto collection related configuration
+export HADOOP_CLASSPATH=`hadoop classpath`:${DOLPHINSCHEDULER_HOME}/tools/libs/*
+export SPARK_DIST_CLASSPATH=$HADOOP_CLASSPATH:$SPARK_DIST_CLASS_PATH
+export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
+export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
+export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
Review Comment:
@ruanwenjun do you have some suggestion?
--
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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006412611
##########
script/env/dolphinscheduler_env.sh:
##########
@@ -33,3 +33,10 @@ export SEATUNNEL_HOME=${SEATUNNEL_HOME:-/opt/soft/seatunnel}
export CHUNJUN_HOME=${CHUNJUN_HOME:-/opt/soft/chunjun}
export PATH=$HADOOP_HOME/bin:$SPARK_HOME/bin:$PYTHON_HOME/bin:$JAVA_HOME/bin:$HIVE_HOME/bin:$FLINK_HOME/bin:$DATAX_HOME/bin:$SEATUNNEL_HOME/bin:$CHUNJUN_HOME/bin:$PATH
+
+# applicationId auto collection related configuration
+export HADOOP_CLASSPATH=`hadoop classpath`:${DOLPHINSCHEDULER_HOME}/tools/libs/*
+export SPARK_DIST_CLASSPATH=$HADOOP_CLASSPATH:$SPARK_DIST_CLASS_PATH
+export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
+export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
+export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
Review Comment:
@caishunfeng
These extra java opts are necessary, do you have any suggestions to place these configuration elsewhere?
--
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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1008884570
##########
script/env/dolphinscheduler_env.sh:
##########
@@ -33,3 +33,10 @@ export SEATUNNEL_HOME=${SEATUNNEL_HOME:-/opt/soft/seatunnel}
export CHUNJUN_HOME=${CHUNJUN_HOME:-/opt/soft/chunjun}
export PATH=$HADOOP_HOME/bin:$SPARK_HOME/bin:$PYTHON_HOME/bin:$JAVA_HOME/bin:$HIVE_HOME/bin:$FLINK_HOME/bin:$DATAX_HOME/bin:$SEATUNNEL_HOME/bin:$CHUNJUN_HOME/bin:$PATH
+
+# applicationId auto collection related configuration
+export HADOOP_CLASSPATH=`hadoop classpath`:${DOLPHINSCHEDULER_HOME}/tools/libs/*
+export SPARK_DIST_CLASSPATH=$HADOOP_CLASSPATH:$SPARK_DIST_CLASS_PATH
+export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
+export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
+export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
Review Comment:
@ruanwenjun This PR has to be merged in this month. Please have a look, 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] hstdream commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
hstdream commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r985069696
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractYarnTask.java:
##########
@@ -107,7 +110,7 @@ public void cancelApplication() throws TaskException {
*/
@Override
public List<String> getApplicationIds() throws TaskException {
- return LogUtils.getAppIdsFromLogFile(taskRequest.getLogPath(), logger);
+ return LogUtils.getAppIds(taskRequest.getLogPath(), taskRequest.getAppInfoPath(), PropertyUtils.getString(APPID_COLLECT, "log"));
Review Comment:
It is recommended that the log string be added to the constant.
--
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] github-code-scanning[bot] commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1007547149
##########
dolphinscheduler-aop/src/test/java/org/apache/dolphinscheduler/YarnClientAspectMocTest.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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;
+
+import org.apache.dolphinscheduler.poc.YarnClientMoc;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+import org.apache.hadoop.yarn.api.records.Priority;
+import org.apache.hadoop.yarn.exceptions.YarnException;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class YarnClientAspectMocTest {
Review Comment:
## Unused classes and interfaces
Unused class: YarnClientAspectMocTest is not referenced within this codebase. If not used as an external API it should be removed.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/2145)
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1294423142
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [22.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006420020
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/task/BaseTaskProcessor.java:
##########
@@ -340,6 +340,7 @@ protected TaskExecutionContext getTaskExecutionContext(TaskInstance taskInstance
.buildTaskDefinitionRelatedInfo(taskInstance.getTaskDefine())
.buildProcessInstanceRelatedInfo(taskInstance.getProcessInstance())
.buildProcessDefinitionRelatedInfo(taskInstance.getProcessDefine())
+ .buildExecPathRelatedInfo()
Review Comment:
Same answer below.
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1289864027
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/12197?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 [#12197](https://codecov.io/gh/apache/dolphinscheduler/pull/12197?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (94dfe46) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/997b022b7cf6d03f8b1b4ac220c58349a553ea2c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (997b022) will **decrease** coverage by `0.05%`.
> The diff coverage is `23.21%`.
```diff
@@ Coverage Diff @@
## dev #12197 +/- ##
============================================
- Coverage 39.61% 39.56% -0.06%
+ Complexity 4192 4191 -1
============================================
Files 1038 1038
Lines 38843 38825 -18
Branches 4447 4435 -12
============================================
- Hits 15388 15361 -27
- Misses 21711 21718 +7
- Partials 1744 1746 +2
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/12197?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/dolphinscheduler/common/constants/Constants.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL2NvbnN0YW50cy9Db25zdGFudHMuamF2YQ==) | `62.50% <ø> (ø)` | |
| [...pache/dolphinscheduler/common/utils/FileUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0ZpbGVVdGlscy5qYXZh) | `54.28% <0.00%> (-0.79%)` | :arrow_down: |
| [...er/master/builder/TaskExecutionContextBuilder.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9idWlsZGVyL1Rhc2tFeGVjdXRpb25Db250ZXh0QnVpbGRlci5qYXZh) | `56.06% <0.00%> (-10.02%)` | :arrow_down: |
| [...r/server/master/runner/task/BaseTaskProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvdGFzay9CYXNlVGFza1Byb2Nlc3Nvci5qYXZh) | `15.75% <0.00%> (-0.06%)` | :arrow_down: |
| [...ler/remote/command/log/GetAppIdRequestCommand.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL2NvbW1hbmQvbG9nL0dldEFwcElkUmVxdWVzdENvbW1hbmQuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...apache/dolphinscheduler/service/log/LogClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvbG9nL0xvZ0NsaWVudC5qYXZh) | `41.00% <0.00%> (-0.42%)` | :arrow_down: |
| [...nscheduler/service/log/LoggerRequestProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvbG9nL0xvZ2dlclJlcXVlc3RQcm9jZXNzb3IuamF2YQ==) | `16.98% <0.00%> (-0.17%)` | :arrow_down: |
| [...e/dolphinscheduler/service/utils/ProcessUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvdXRpbHMvUHJvY2Vzc1V0aWxzLmphdmE=) | `27.65% <0.00%> (-0.30%)` | :arrow_down: |
| [...hinscheduler/plugin/task/api/AbstractYarnTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL2FwaS9BYnN0cmFjdFlhcm5UYXNrLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ler/server/worker/processor/TaskKillProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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-ZG9scGhpbnNjaGVkdWxlci13b3JrZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci9wcm9jZXNzb3IvVGFza0tpbGxQcm9jZXNzb3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| ... and [35 more](https://codecov.io/gh/apache/dolphinscheduler/pull/12197/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) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1007685050
##########
script/env/dolphinscheduler_env.sh:
##########
@@ -33,3 +33,10 @@ export SEATUNNEL_HOME=${SEATUNNEL_HOME:-/opt/soft/seatunnel}
export CHUNJUN_HOME=${CHUNJUN_HOME:-/opt/soft/chunjun}
export PATH=$HADOOP_HOME/bin:$SPARK_HOME/bin:$PYTHON_HOME/bin:$JAVA_HOME/bin:$HIVE_HOME/bin:$FLINK_HOME/bin:$DATAX_HOME/bin:$SEATUNNEL_HOME/bin:$CHUNJUN_HOME/bin:$PATH
+
+# applicationId auto collection related configuration
+export HADOOP_CLASSPATH=`hadoop classpath`:${DOLPHINSCHEDULER_HOME}/tools/libs/*
+export SPARK_DIST_CLASSPATH=$HADOOP_CLASSPATH:$SPARK_DIST_CLASS_PATH
+export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
+export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
+export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
Review Comment:
@caishunfeng Please have a look at the latest change, include most of your suggestions
##########
script/env/dolphinscheduler_env.sh:
##########
@@ -33,3 +33,10 @@ export SEATUNNEL_HOME=${SEATUNNEL_HOME:-/opt/soft/seatunnel}
export CHUNJUN_HOME=${CHUNJUN_HOME:-/opt/soft/chunjun}
export PATH=$HADOOP_HOME/bin:$SPARK_HOME/bin:$PYTHON_HOME/bin:$JAVA_HOME/bin:$HIVE_HOME/bin:$FLINK_HOME/bin:$DATAX_HOME/bin:$SEATUNNEL_HOME/bin:$CHUNJUN_HOME/bin:$PATH
+
+# applicationId auto collection related configuration
+export HADOOP_CLASSPATH=`hadoop classpath`:${DOLPHINSCHEDULER_HOME}/tools/libs/*
+export SPARK_DIST_CLASSPATH=$HADOOP_CLASSPATH:$SPARK_DIST_CLASS_PATH
+export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
+export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
+export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
Review Comment:
@caishunfeng Please have a look at the latest changes, include most of your suggestions
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1005266248
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private final String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = String.format("%s/%s", System.getProperty("user.dir"), "appInfo.log");
+ debug = true;
+ }
+
+ /**
+ * Trigger submitApplication when invoking YarnClientImpl.submitApplication
+ *
+ * @param appContext application context when invoking YarnClientImpl.submitApplication
+ * @param submittedAppId the submitted application id returned by YarnClientImpl.submitApplication
+ * @throws Throwable exceptions
+ */
+ @AfterReturning(pointcut = "execution(ApplicationId org.apache.hadoop.yarn.client.api.impl.YarnClientImpl." +
+ "submitApplication(ApplicationSubmissionContext)) && args(appContext)", returning = "submittedAppId", argNames = "appContext,submittedAppId")
+ public void registerApplicationInfo(ApplicationSubmissionContext appContext, ApplicationId submittedAppId) {
+ if (appInfoFilePath != null) {
+ try {
+ Files.write(Paths.get(appInfoFilePath),
+ Collections.singletonList(submittedAppId.toString()),
+ StandardOpenOption.CREATE,
+ StandardOpenOption.WRITE,
+ StandardOpenOption.APPEND);
+ } catch (IOException ioException) {
+ System.err.println(
+ "YarnClientAspect[registerAppInfo]: can't output current application information, because "
+ + ioException.getMessage());
Review Comment:
Please using `logger.error` instead of stdout.
##########
dolphinscheduler-aop/src/test/java/org/apache/dolphinscheduler/YarnClientAspectMocTest.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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;
+
+import org.apache.dolphinscheduler.poc.YarnClientMoc;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+import org.apache.hadoop.yarn.api.records.Priority;
+import org.apache.hadoop.yarn.exceptions.YarnException;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class YarnClientAspectMocTest {
+
+ private final PrintStream standardOut = System.out;
+ ByteArrayOutputStream stdoutStream = new ByteArrayOutputStream();
+ @BeforeEach
+ public void beforeEveryTest() {
+ System.setOut(new PrintStream(stdoutStream));
+ }
+ @AfterEach
+ public void afterEveryTest() throws IOException {
+ System.setOut(standardOut);
+ stdoutStream.close();
+ }
+ @Test
+ public void testMoc() {
+ YarnClientMoc moc = new YarnClientMoc();
+ try {
+ ApplicationSubmissionContext appContext = ApplicationSubmissionContext.newInstance(
+ ApplicationId.newInstance(System.currentTimeMillis(), 1236), "appName",
+ "queue", Priority.UNDEFINED,
+ null, false,
+ false, 10, null,
+ "type");
+ moc.createAppId();
+ ApplicationId applicationId = moc.submitApplication(appContext);
+ String stdoutContent = stdoutStream.toString();
+ Assertions.assertTrue(stdoutContent.contains("YarnClientAspectMoc[submitApplication]"),
+ "trigger YarnClientAspectMoc.submitApplication failed");
+ Assertions.assertTrue(stdoutContent.contains("YarnClientAspectMoc[createAppId]:"),
+ "trigger YarnClientAspectMoc.createAppId failed");
+ } catch (YarnException | IOException e) {
+ Assertions.fail("test YarnClientAspectMoc failed: " + e.getMessage());
+ e.printStackTrace();
Review Comment:
Please using `logger.error` instead of this.
##########
dolphinscheduler-aop/src/test/java/org/apache/dolphinscheduler/poc/YarnClientMoc.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.poc;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+import org.apache.hadoop.yarn.exceptions.YarnException;
+
+import java.io.IOException;
+import java.util.Random;
+
+public class YarnClientMoc {
+
+ private Random random = new Random();
+
+ public ApplicationId createAppId() {
+ ApplicationId created = ApplicationId.newInstance(System.currentTimeMillis(), random.nextInt());
+ System.out.println("created id " + created.getId());
Review Comment:
Please using `logger.info` instead of this.
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private final String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = String.format("%s/%s", System.getProperty("user.dir"), "appInfo.log");
+ debug = true;
+ }
+
+ /**
+ * Trigger submitApplication when invoking YarnClientImpl.submitApplication
+ *
+ * @param appContext application context when invoking YarnClientImpl.submitApplication
+ * @param submittedAppId the submitted application id returned by YarnClientImpl.submitApplication
+ * @throws Throwable exceptions
+ */
+ @AfterReturning(pointcut = "execution(ApplicationId org.apache.hadoop.yarn.client.api.impl.YarnClientImpl." +
+ "submitApplication(ApplicationSubmissionContext)) && args(appContext)", returning = "submittedAppId", argNames = "appContext,submittedAppId")
+ public void registerApplicationInfo(ApplicationSubmissionContext appContext, ApplicationId submittedAppId) {
+ if (appInfoFilePath != null) {
+ try {
+ Files.write(Paths.get(appInfoFilePath),
+ Collections.singletonList(submittedAppId.toString()),
+ StandardOpenOption.CREATE,
+ StandardOpenOption.WRITE,
+ StandardOpenOption.APPEND);
+ } catch (IOException ioException) {
+ System.err.println(
+ "YarnClientAspect[registerAppInfo]: can't output current application information, because "
+ + ioException.getMessage());
+ }
+ }
+ if (debug) {
+ System.out.println("YarnClientAspect[submitApplication]: current application context " + appContext);
+ System.out.println("YarnClientAspect[submitApplication]: submitted application id " + submittedAppId);
+ System.out.println(
Review Comment:
Same 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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r985111115
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractYarnTask.java:
##########
@@ -107,7 +110,7 @@ public void cancelApplication() throws TaskException {
*/
@Override
public List<String> getApplicationIds() throws TaskException {
- return LogUtils.getAppIdsFromLogFile(taskRequest.getLogPath(), logger);
+ return LogUtils.getAppIds(taskRequest.getLogPath(), taskRequest.getAppInfoPath(), PropertyUtils.getString(APPID_COLLECT, "log"));
Review Comment:
Thanks for reminding!
--
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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983762859
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * flag to indicate whether print debug logs
+ */
+ private static final String PARA_NAME_ASPECTJ_DEBUG = "PARA_NAME_ASPECTJ_DEBUG";
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = System.getProperty("user.dir") + "/appInfo.log";
+ debug = Boolean.parseBoolean(System.getenv(PARA_NAME_ASPECTJ_DEBUG));
Review Comment:
`PARA_NAME_ASPECTJ_DEBUG` is now configured in `dolphinscheduler_env.sh`. Actually, i'm considering about the configuration `appId.file.path` which i've already added into common.properties(default value is '/appInfo.log'), however, it's hard coded now in the above aop code. I'll try to optimize this part of code.
--
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] fuchanghai commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
fuchanghai commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983136317
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * flag to indicate whether print debug logs
+ */
+ private static final String PARA_NAME_ASPECTJ_DEBUG = "PARA_NAME_ASPECTJ_DEBUG";
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = System.getProperty("user.dir") + "/appInfo.log";
+ debug = Boolean.parseBoolean(System.getenv(PARA_NAME_ASPECTJ_DEBUG));
+ }
+
+ /**
+ * Trigger submitApplication when invoking YarnClientImpl.submitApplication
+ *
+ * @param appContext application context when invoking YarnClientImpl.submitApplication
+ * @param submittedAppId the submitted application id returned by YarnClientImpl.submitApplication
+ * @throws Throwable exceptions
+ */
+ @AfterReturning(pointcut = "execution(ApplicationId org.apache.hadoop.yarn.client.api.impl.YarnClientImpl." +
+ "submitApplication(ApplicationSubmissionContext)) && args(appContext)",
+ returning = "submittedAppId", argNames = "appContext,submittedAppId")
+ public void registerApplicationInfo(ApplicationSubmissionContext appContext, ApplicationId submittedAppId) {
+ if (appInfoFilePath != null) {
+ try {
+ Files.write(Paths.get(appInfoFilePath),
+ Collections.singletonList(submittedAppId.toString()),
+ StandardOpenOption.CREATE,
+ StandardOpenOption.WRITE,
+ StandardOpenOption.APPEND);
+ } catch (IOException ioException) {
+ System.out.println(
Review Comment:
maybe we should replace ```System.out``` with ```@Slf4j```
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -75,14 +75,14 @@ public static void checkTenantExist(WorkerConfig workerConfig, TaskExecutionCont
public static void createProcessLocalPathIfAbsent(TaskExecutionContext taskExecutionContext) throws TaskException {
try {
// local execute path
- String execLocalPath = FileUtils.getProcessExecDir(
- taskExecutionContext.getProjectCode(),
- taskExecutionContext.getProcessDefineCode(),
- taskExecutionContext.getProcessDefineVersion(),
- taskExecutionContext.getProcessInstanceId(),
- taskExecutionContext.getTaskInstanceId());
- taskExecutionContext.setExecutePath(execLocalPath);
- FileUtils.createWorkDirIfAbsent(execLocalPath);
+ // String execLocalPath = FileUtils.getProcessExecDir(
+ // taskExecutionContext.getProjectCode(),
+ // taskExecutionContext.getProcessDefineCode(),
+ // taskExecutionContext.getProcessDefineVersion(),
+ // taskExecutionContext.getProcessInstanceId(),
+ // taskExecutionContext.getTaskInstanbu iceId());
+ // taskExecutionContext.setExecutePath(execLocalPath);
+ FileUtils.createWorkDirIfAbsent(taskExecutionContext.getExecutePath());
Review Comment:
If you leave this part out, where did you set the ```execLocalPath```?
please tell me where it is set, maybe I didn't notice 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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006419589
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/TaskExecutionContext.java:
##########
@@ -81,6 +81,11 @@ public class TaskExecutionContext implements Serializable {
*/
private String logPath;
+ /**
+ * applicationId path
+ */
+ private String appInfoPath;
Review Comment:
@caishunfeng
You're right, no need, but it just seems more clear. Personally, I prefer to keep it, WDYT?
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006522382
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/builder/TaskExecutionContextBuilder.java:
##########
@@ -126,6 +127,24 @@ public TaskExecutionContextBuilder buildProcessDefinitionRelatedInfo(ProcessDefi
return this;
}
+ /**
+ * build execPath related info
+ *
+ * @return TaskExecutionContextBuilder
+ */
+ public TaskExecutionContextBuilder buildExecPathRelatedInfo() {
Review Comment:
> I think it should setExecutePath and appInfoPath in worker before task running. cc @ruanwenjun
Yes, it should be move to worker, and should be set before yarn task running, I am not sure if other task will need this,
you can simply put this before all task running.
@Radeity In fact, the master doesn't know execPath, it is defined in worker, different worker may have different path, master doesn't know the execpath
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/builder/TaskExecutionContextBuilder.java:
##########
@@ -126,6 +127,24 @@ public TaskExecutionContextBuilder buildProcessDefinitionRelatedInfo(ProcessDefi
return this;
}
+ /**
+ * build execPath related info
+ *
+ * @return TaskExecutionContextBuilder
+ */
+ public TaskExecutionContextBuilder buildExecPathRelatedInfo() {
Review Comment:
> I think it should setExecutePath and appInfoPath in worker before task running. cc @ruanwenjun
Yes, it should be move to worker, and should be set before yarn task running, I am not sure if other task will need this,
you can simply put this before all task running.
@Radeity In fact, the master doesn't know execPath, it is defined in worker, different worker may have different path, master doesn't know the execpath
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1296654839
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [39 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [45.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [2.1% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1008884570
##########
script/env/dolphinscheduler_env.sh:
##########
@@ -33,3 +33,10 @@ export SEATUNNEL_HOME=${SEATUNNEL_HOME:-/opt/soft/seatunnel}
export CHUNJUN_HOME=${CHUNJUN_HOME:-/opt/soft/chunjun}
export PATH=$HADOOP_HOME/bin:$SPARK_HOME/bin:$PYTHON_HOME/bin:$JAVA_HOME/bin:$HIVE_HOME/bin:$FLINK_HOME/bin:$DATAX_HOME/bin:$SEATUNNEL_HOME/bin:$CHUNJUN_HOME/bin:$PATH
+
+# applicationId auto collection related configuration
+export HADOOP_CLASSPATH=`hadoop classpath`:${DOLPHINSCHEDULER_HOME}/tools/libs/*
+export SPARK_DIST_CLASSPATH=$HADOOP_CLASSPATH:$SPARK_DIST_CLASS_PATH
+export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
+export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
+export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
Review Comment:
@ruanwenjun This PR has to be merged in this month. Please have a look, 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] EricGao888 commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
EricGao888 commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983306935
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -119,7 +126,7 @@ public static String getResourceViewSuffixes() {
* @throws IOException errors
*/
public static void createWorkDirIfAbsent(String execLocalPath) throws IOException {
- //if work dir exists, first delete
+ // if work dir exists, first delete
Review Comment:
I'd rather we just remove this line of comment. The code has indicated what it is doing here. BTW, this comment has already been away from the code, which also shows it is hard to maintain such kind of comment.
```java
if (execLocalPathFile.exists()) {
try {
org.apache.commons.io.FileUtils.forceDelete(execLocalPathFile);
......
```
--
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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983762859
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * flag to indicate whether print debug logs
+ */
+ private static final String PARA_NAME_ASPECTJ_DEBUG = "PARA_NAME_ASPECTJ_DEBUG";
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = System.getProperty("user.dir") + "/appInfo.log";
+ debug = Boolean.parseBoolean(System.getenv(PARA_NAME_ASPECTJ_DEBUG));
Review Comment:
> if he depends on the common module, it can be solved. I think this variable must be configurable and cannot be configured by modifying the code. if above cann't fits your needs.We should prioritize using the configuration file in this module. Welcome to give your feedback any comments
> if he depends on the common module, it can be solved. I think this variable must be configurable and cannot be configured by modifying the code. if above cann't fits your needs.We should prioritize using the configuration file in this module. Welcome to give your feedback any comments
`PARA_NAME_ASPECTJ_DEBUG` is now configured in `dolphinscheduler_env.sh`. Actually, i'm considering about the configuration `appId.file.path` which i've already added into common.properties(default value is '/appInfo.log'), however, it's hard coded now in the above aop code. I'll try to optimize this part of code.
--
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] fuchanghai commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
fuchanghai commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983113647
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * flag to indicate whether print debug logs
+ */
+ private static final String PARA_NAME_ASPECTJ_DEBUG = "PARA_NAME_ASPECTJ_DEBUG";
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = System.getProperty("user.dir") + "/appInfo.log";
+ debug = Boolean.parseBoolean(System.getenv(PARA_NAME_ASPECTJ_DEBUG));
Review Comment:
should we put this variable ```PARA_NAME_ASPECTJ_DEBUG``` into common.properties?
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -119,7 +126,7 @@ public static String getResourceViewSuffixes() {
* @throws IOException errors
*/
public static void createWorkDirIfAbsent(String execLocalPath) throws IOException {
- //if work dir exists, first delete
+ // if work dir exists, first delete
Review Comment:
```work dir``` is puzzling ，maybe we can replace that with ```work catalog``` or ```work directory```? @EricGao888 PTAL
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -17,12 +17,7 @@
package org.apache.dolphinscheduler.common.utils;
-import static org.apache.dolphinscheduler.common.Constants.DATA_BASEDIR_PATH;
-import static org.apache.dolphinscheduler.common.Constants.FOLDER_SEPARATOR;
-import static org.apache.dolphinscheduler.common.Constants.RESOURCE_VIEW_SUFFIXES;
-import static org.apache.dolphinscheduler.common.Constants.RESOURCE_VIEW_SUFFIXES_DEFAULT_VALUE;
-import static org.apache.dolphinscheduler.common.Constants.UTF_8;
-import static org.apache.dolphinscheduler.common.Constants.YYYYMMDDHHMMSS;
+import static org.apache.dolphinscheduler.common.Constants.*;
Review Comment:
should we avoid to use ```.*```?
--
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] fuchanghai commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
fuchanghai commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983568190
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -75,14 +75,14 @@ public static void checkTenantExist(WorkerConfig workerConfig, TaskExecutionCont
public static void createProcessLocalPathIfAbsent(TaskExecutionContext taskExecutionContext) throws TaskException {
try {
// local execute path
- String execLocalPath = FileUtils.getProcessExecDir(
- taskExecutionContext.getProjectCode(),
- taskExecutionContext.getProcessDefineCode(),
- taskExecutionContext.getProcessDefineVersion(),
- taskExecutionContext.getProcessInstanceId(),
- taskExecutionContext.getTaskInstanceId());
- taskExecutionContext.setExecutePath(execLocalPath);
- FileUtils.createWorkDirIfAbsent(execLocalPath);
+ // String execLocalPath = FileUtils.getProcessExecDir(
+ // taskExecutionContext.getProjectCode(),
+ // taskExecutionContext.getProcessDefineCode(),
+ // taskExecutionContext.getProcessDefineVersion(),
+ // taskExecutionContext.getProcessInstanceId(),
+ // taskExecutionContext.getTaskInstanbu iceId());
+ // taskExecutionContext.setExecutePath(execLocalPath);
+ FileUtils.createWorkDirIfAbsent(taskExecutionContext.getExecutePath());
Review Comment:
> Yes, if it's not yarn job, aop code will not execute. However, all types of tasks will generate execution path (as well as appInfoPath) in master, so i don't get what you mean "impossible to get the execution path", would you like to explain it in detail?
@Radeity sorry ,I found where you are setting the value. I have no doubts about this piece of code
--
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] github-code-scanning[bot] commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1000872353
##########
dolphinscheduler-aop/src/test/java/org/apache/dolphinscheduler/poc/YarnClientMoc.java:
##########
@@ -0,0 +1,23 @@
+package org.apache.dolphinscheduler.poc;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+import org.apache.hadoop.yarn.exceptions.YarnException;
+
+import java.io.IOException;
+import java.util.Random;
+
+public class YarnClientMoc {
+
+ private Random random = new Random();
+
+ public ApplicationId createAppId() {
+ ApplicationId created = ApplicationId.newInstance(System.currentTimeMillis(), random.nextInt());
+ System.out.println("created id " + created.getId());
+ return created;
+ }
+
+ public ApplicationId submitApplication(ApplicationSubmissionContext appContext) throws YarnException, IOException {
Review Comment:
## Useless parameter
The parameter 'appContext' is never used.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1996)
##########
dolphinscheduler-aop/src/test/java/org/apache/dolphinscheduler/YarnClientAspectMocTest.java:
##########
@@ -0,0 +1,54 @@
+package org.apache.dolphinscheduler;
+
+import org.apache.dolphinscheduler.poc.YarnClientMoc;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+import org.apache.hadoop.yarn.api.records.Priority;
+import org.apache.hadoop.yarn.exceptions.YarnException;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class YarnClientAspectMocTest {
+
+ private final PrintStream standardOut = System.out;
+ ByteArrayOutputStream stdoutStream = new ByteArrayOutputStream();
+ @BeforeEach
+ public void beforeEveryTest() {
+ System.setOut(new PrintStream(stdoutStream));
+ }
+ @AfterEach
+ public void afterEveryTest() throws IOException {
+ System.setOut(standardOut);
+ stdoutStream.close();
+ }
+ @Test
+ public void testMoc() {
+ YarnClientMoc moc = new YarnClientMoc();
+ try {
+ ApplicationSubmissionContext appContext = ApplicationSubmissionContext.newInstance(
+ ApplicationId.newInstance(System.currentTimeMillis(), 1236), "appName",
+ "queue", Priority.UNDEFINED,
+ null, false,
+ false, 10, null,
+ "type");
+ moc.createAppId();
+ ApplicationId applicationId = moc.submitApplication(appContext);
Review Comment:
## Unread local variable
Variable 'ApplicationId applicationId' is never read.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1997)
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1289868314
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [21.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006418272
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/builder/TaskExecutionContextBuilder.java:
##########
@@ -126,6 +127,24 @@ public TaskExecutionContextBuilder buildProcessDefinitionRelatedInfo(ProcessDefi
return this;
}
+ /**
+ * build execPath related info
+ *
+ * @return TaskExecutionContextBuilder
+ */
+ public TaskExecutionContextBuilder buildExecPathRelatedInfo() {
Review Comment:
@caishunfeng
I think whether `execPath` or `appInfoPath` is context-related, and will never change, it's weird to set it in worker, so i put it forward in master.
--
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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1008075736
##########
script/env/dolphinscheduler_env.sh:
##########
@@ -33,3 +33,10 @@ export SEATUNNEL_HOME=${SEATUNNEL_HOME:-/opt/soft/seatunnel}
export CHUNJUN_HOME=${CHUNJUN_HOME:-/opt/soft/chunjun}
export PATH=$HADOOP_HOME/bin:$SPARK_HOME/bin:$PYTHON_HOME/bin:$JAVA_HOME/bin:$HIVE_HOME/bin:$FLINK_HOME/bin:$DATAX_HOME/bin:$SEATUNNEL_HOME/bin:$CHUNJUN_HOME/bin:$PATH
+
+# applicationId auto collection related configuration
+export HADOOP_CLASSPATH=`hadoop classpath`:${DOLPHINSCHEDULER_HOME}/tools/libs/*
+export SPARK_DIST_CLASSPATH=$HADOOP_CLASSPATH:$SPARK_DIST_CLASS_PATH
+export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
+export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
+export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
Review Comment:
@ruanwenjun Any suggestions will be appreciated! It seems that everything is okay except for this part.
--
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] gabrywu commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
gabrywu commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r994377909
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.aop;
+
+import static org.apache.dolphinscheduler.common.Constants.*;
+
+import org.apache.dolphinscheduler.common.utils.PropertyUtils;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ // public static final Logger logger = LoggerFactory.getLogger(YarnClientAspect.class);
Review Comment:
please delete unused codes
##########
docs/docs/en/architecture/configuration.md:
##########
@@ -224,6 +224,9 @@ The default configuration is as follows:
|sudo.enable | true | whether to enable sudo|
|alert.rpc.port | 50052 | the RPC port of Alert Server|
|zeppelin.rest.url | http://localhost:8080 | the RESTful API url of zeppelin|
+|appId.collect | log | way to collect applicationId, if use aop, alter the configuration from log to aop|
+|appId.file.path | appInfo.log | if use aop way，the relative log path to store applicationId (suggest not to change, need to re-package aop jar file)|
Review Comment:
If we can't change this config except that we re-build the jar, we'd better create a static field in a class. If user can build the jar, a static field is enough to change as users want.
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.aop;
+
+import static org.apache.dolphinscheduler.common.Constants.*;
+
+import org.apache.dolphinscheduler.common.utils.PropertyUtils;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ // public static final Logger logger = LoggerFactory.getLogger(YarnClientAspect.class);
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private String appInfoFilePath;
Review Comment:
should be a final field
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * flag to indicate whether print debug logs
+ */
+ private static final String PARA_NAME_ASPECTJ_DEBUG = "PARA_NAME_ASPECTJ_DEBUG";
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = System.getProperty("user.dir") + "/appInfo.log";
+ debug = Boolean.parseBoolean(System.getenv(PARA_NAME_ASPECTJ_DEBUG));
+ }
+
+ /**
+ * Trigger submitApplication when invoking YarnClientImpl.submitApplication
+ *
+ * @param appContext application context when invoking YarnClientImpl.submitApplication
+ * @param submittedAppId the submitted application id returned by YarnClientImpl.submitApplication
+ * @throws Throwable exceptions
+ */
+ @AfterReturning(pointcut = "execution(ApplicationId org.apache.hadoop.yarn.client.api.impl.YarnClientImpl." +
+ "submitApplication(ApplicationSubmissionContext)) && args(appContext)",
+ returning = "submittedAppId", argNames = "appContext,submittedAppId")
+ public void registerApplicationInfo(ApplicationSubmissionContext appContext, ApplicationId submittedAppId) {
+ if (appInfoFilePath != null) {
+ try {
+ Files.write(Paths.get(appInfoFilePath),
+ Collections.singletonList(submittedAppId.toString()),
+ StandardOpenOption.CREATE,
+ StandardOpenOption.WRITE,
+ StandardOpenOption.APPEND);
+ } catch (IOException ioException) {
+ System.out.println(
Review Comment:
System.err is another option
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/LogUtils.java:
##########
@@ -44,10 +44,38 @@ public class LogUtils {
private static final Pattern APPLICATION_REGEX = Pattern.compile(TaskConstants.YARN_APPLICATION_REGEX);
- public List<String> getAppIdsFromLogFile(@NonNull String logPath) {
- return getAppIdsFromLogFile(logPath, log);
+ public List<String> getAppIds(@NonNull String logPath, @NonNull String appInfoPath, String fetchWay) {
+ switch (fetchWay) {
+ case "aop":
+ log.info("Start finding appId in {}, fetch way: {} ", appInfoPath);
+ return getAppIdsFromAppInfoFile(appInfoPath, log);
+ case "log":
+ log.info("Start finding appId in {}, fetch way: {} ", logPath);
+ return getAppIdsFromLogFile(logPath, log);
+ default:
Review Comment:
default is `log`
##########
dolphinscheduler-api-test/dolphinscheduler-api-test-case/src/test/resources/docker/file-manage/common.properties:
##########
@@ -15,7 +15,7 @@
# limitations under the License.
#
# user data local directory path, please make sure the directory exists and have read write permissions
-data.basedir.path=/tmp/dolphinscheduler
+data.basedir.path=/home/wangwr/tmp/dolphinscheduler
Review Comment:
need to change 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] caishunfeng commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006373010
##########
dolphinscheduler-aop/src/test/java/org/apache/dolphinscheduler/poc/YarnClientAspectMoc.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.poc;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Aspect
+public class YarnClientAspectMoc {
+
+ protected final Logger logger = LoggerFactory.getLogger(getClass());
+
+ private ApplicationId privateId = null;
+
+ @AfterReturning(pointcut = "execution(ApplicationId org.apache.dolphinscheduler.poc.YarnClientMoc.submitApplication(ApplicationSubmissionContext)) && args(appContext)", returning = "submittedAppId", argNames = "appContext")
+ public void submitApplication(ApplicationSubmissionContext appContext, ApplicationId submittedAppId) {
+ logger.info("YarnClientAspectMoc[submitApplication]: app context " + appContext + ", submittedAppId "
+ + submittedAppId + " privateId " + privateId);
Review Comment:
```suggestion
logger.info("YarnClientAspectMoc[submitApplication]: app context: {}, submittedAppId: {}, privateId: {}", appContext, submittedAppId, privateId);
```
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private final String appInfoFilePath;
+ private boolean debug;
+
+ protected final Logger logger = LoggerFactory.getLogger(getClass());
+
+ public YarnClientAspect() {
+ appInfoFilePath = String.format("%s/%s", System.getProperty("user.dir"), "appInfo.log");
+ debug = true;
Review Comment:
```suggestion
```
##########
dolphinscheduler-aop/src/test/java/org/apache/dolphinscheduler/poc/YarnClientMoc.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.poc;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+import org.apache.hadoop.yarn.exceptions.YarnException;
+
+import java.io.IOException;
+import java.util.Random;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class YarnClientMoc {
+
+ protected final Logger logger = LoggerFactory.getLogger(getClass());
+
+ private Random random = new Random();
+
+ public ApplicationId createAppId() {
+ ApplicationId created = ApplicationId.newInstance(System.currentTimeMillis(), random.nextInt());
+ logger.info("created id " + created.getId());
Review Comment:
```suggestion
logger.info("created id {}", created.getId());
```
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private final String appInfoFilePath;
+ private boolean debug;
+
+ protected final Logger logger = LoggerFactory.getLogger(getClass());
+
+ public YarnClientAspect() {
+ appInfoFilePath = String.format("%s/%s", System.getProperty("user.dir"), "appInfo.log");
+ debug = true;
+ }
+
+ /**
+ * Trigger submitApplication when invoking YarnClientImpl.submitApplication
+ *
+ * @param appContext application context when invoking YarnClientImpl.submitApplication
+ * @param submittedAppId the submitted application id returned by YarnClientImpl.submitApplication
+ * @throws Throwable exceptions
+ */
+ @AfterReturning(pointcut = "execution(ApplicationId org.apache.hadoop.yarn.client.api.impl.YarnClientImpl." +
+ "submitApplication(ApplicationSubmissionContext)) && args(appContext)", returning = "submittedAppId", argNames = "appContext,submittedAppId")
+ public void registerApplicationInfo(ApplicationSubmissionContext appContext, ApplicationId submittedAppId) {
+ if (appInfoFilePath != null) {
+ try {
+ Files.write(Paths.get(appInfoFilePath),
+ Collections.singletonList(submittedAppId.toString()),
+ StandardOpenOption.CREATE,
+ StandardOpenOption.WRITE,
+ StandardOpenOption.APPEND);
+ } catch (IOException ioException) {
+ logger.error("YarnClientAspect[registerAppInfo]: can't output current application information, because "
+ + ioException.getMessage());
+ }
+ }
+ if (debug) {
+ logger.info("YarnClientAspect[submitApplication]: current application context " + appContext);
+ logger.info("YarnClientAspect[submitApplication]: submitted application id " + submittedAppId);
+ logger.info(
+ "YarnClientAspect[submitApplication]: current application report " + currentApplicationReport);
+ }
Review Comment:
```suggestion
logger.info("YarnClientAspect[submitApplication]: current application context " + appContext);
logger.info("YarnClientAspect[submitApplication]: submitted application id " + submittedAppId);
logger.info(
"YarnClientAspect[submitApplication]: current application report " + currentApplicationReport);
```
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/task/BaseTaskProcessor.java:
##########
@@ -340,6 +340,7 @@ protected TaskExecutionContext getTaskExecutionContext(TaskInstance taskInstance
.buildTaskDefinitionRelatedInfo(taskInstance.getTaskDefine())
.buildProcessInstanceRelatedInfo(taskInstance.getProcessInstance())
.buildProcessDefinitionRelatedInfo(taskInstance.getProcessDefine())
+ .buildExecPathRelatedInfo()
Review Comment:
Why to `buildExecPathRelatedInfo` here?
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/builder/TaskExecutionContextBuilder.java:
##########
@@ -126,6 +127,24 @@ public TaskExecutionContextBuilder buildProcessDefinitionRelatedInfo(ProcessDefi
return this;
}
+ /**
+ * build execPath related info
+ *
+ * @return TaskExecutionContextBuilder
+ */
+ public TaskExecutionContextBuilder buildExecPathRelatedInfo() {
Review Comment:
I think it should setExecutePath and appInfoPath in worker before task running. cc @ruanwenjun
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/TaskExecutionContext.java:
##########
@@ -81,6 +81,11 @@ public class TaskExecutionContext implements Serializable {
*/
private String logPath;
+ /**
+ * applicationId path
+ */
+ private String appInfoPath;
Review Comment:
It's no need to add this field if the appInfoPath is default `execPath/appInfo.log`.
##########
script/env/dolphinscheduler_env.sh:
##########
@@ -33,3 +33,10 @@ export SEATUNNEL_HOME=${SEATUNNEL_HOME:-/opt/soft/seatunnel}
export CHUNJUN_HOME=${CHUNJUN_HOME:-/opt/soft/chunjun}
export PATH=$HADOOP_HOME/bin:$SPARK_HOME/bin:$PYTHON_HOME/bin:$JAVA_HOME/bin:$HIVE_HOME/bin:$FLINK_HOME/bin:$DATAX_HOME/bin:$SEATUNNEL_HOME/bin:$CHUNJUN_HOME/bin:$PATH
+
+# applicationId auto collection related configuration
+export HADOOP_CLASSPATH=`hadoop classpath`:${DOLPHINSCHEDULER_HOME}/tools/libs/*
+export SPARK_DIST_CLASSPATH=$HADOOP_CLASSPATH:$SPARK_DIST_CLASS_PATH
+export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
+export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
+export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
Review Comment:
It looks weird :thinking: , do we have to add this OPTS here?
##########
dolphinscheduler-aop/src/test/java/org/apache/dolphinscheduler/poc/YarnClientAspectMoc.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.poc;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Aspect
+public class YarnClientAspectMoc {
+
+ protected final Logger logger = LoggerFactory.getLogger(getClass());
+
+ private ApplicationId privateId = null;
+
+ @AfterReturning(pointcut = "execution(ApplicationId org.apache.dolphinscheduler.poc.YarnClientMoc.submitApplication(ApplicationSubmissionContext)) && args(appContext)", returning = "submittedAppId", argNames = "appContext")
+ public void submitApplication(ApplicationSubmissionContext appContext, ApplicationId submittedAppId) {
+ logger.info("YarnClientAspectMoc[submitApplication]: app context " + appContext + ", submittedAppId "
+ + submittedAppId + " privateId " + privateId);
+ }
+
+ @AfterReturning(pointcut = "cflow(execution(ApplicationId org.apache.dolphinscheduler.poc.YarnClientMoc.submitApplication(ApplicationSubmissionContext))) "
+ +
+ "&& !within(CfowAspect) && execution(ApplicationId org.apache.dolphinscheduler.poc.YarnClientMoc.createAppId())", returning = "submittedAppId")
+ public void createAppId(ApplicationId submittedAppId) {
+ privateId = submittedAppId;
+ logger.info("YarnClientAspectMoc[createAppId]: created submittedAppId " + submittedAppId);
Review Comment:
```suggestion
logger.info("YarnClientAspectMoc[createAppId]: created submittedAppId: {}", submittedAppId);
```
##########
dolphinscheduler-aop/pom.xml:
##########
@@ -0,0 +1,91 @@
+<?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.
+ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.dolphinscheduler</groupId>
+ <artifactId>dolphinscheduler</artifactId>
+ <version>dev-SNAPSHOT</version>
+ </parent>
+ <artifactId>dolphinscheduler-aop</artifactId>
+ <packaging>jar</packaging>
+ <name>${project.artifactId}</name>
+ <description>aop 4 YarnClient to get application id when submitting jars using 'yarn jar mainClass args'</description>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>1.8</maven.compiler.source>
+ <maven.compiler.target>1.8</maven.compiler.target>
+ <aspectj.version>1.9.7</aspectj.version>
+ <hadoop.version>3.2.4</hadoop.version>
Review Comment:
Should move to bom module.
##########
dolphinscheduler-aop/src/test/java/org/apache/dolphinscheduler/YarnClientAspectMocTest.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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;
+
+import org.apache.dolphinscheduler.poc.YarnClientMoc;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+import org.apache.hadoop.yarn.api.records.Priority;
+import org.apache.hadoop.yarn.exceptions.YarnException;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class YarnClientAspectMocTest {
+
+ private final PrintStream standardOut = System.out;
+ ByteArrayOutputStream stdoutStream = new ByteArrayOutputStream();
+ @BeforeEach
+ public void beforeEveryTest() {
+ System.setOut(new PrintStream(stdoutStream));
+ }
+ @AfterEach
+ public void afterEveryTest() throws IOException {
+ System.setOut(standardOut);
+ stdoutStream.close();
+ }
+ @Test
+ public void testMoc() {
+ YarnClientMoc moc = new YarnClientMoc();
+ try {
+ ApplicationSubmissionContext appContext = ApplicationSubmissionContext.newInstance(
+ ApplicationId.newInstance(System.currentTimeMillis(), 1236), "appName",
+ "queue", Priority.UNDEFINED,
+ null, false,
+ false, 10, null,
+ "type");
+ moc.createAppId();
+ ApplicationId applicationId = moc.submitApplication(appContext);
+ String stdoutContent = stdoutStream.toString();
+ Assertions.assertTrue(stdoutContent.contains("YarnClientAspectMoc[submitApplication]"),
+ "trigger YarnClientAspectMoc.submitApplication failed");
+ Assertions.assertTrue(stdoutContent.contains("YarnClientAspectMoc[createAppId]:"),
+ "trigger YarnClientAspectMoc.createAppId failed");
+ } catch (YarnException | IOException e) {
+ Assertions.fail("test YarnClientAspectMoc failed: " + e.getMessage());
+ e.printStackTrace();
Review Comment:
Please remove `e.printStackTrace();`
--
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] gabrywu merged pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
gabrywu merged PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197
--
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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983378392
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -75,14 +75,14 @@ public static void checkTenantExist(WorkerConfig workerConfig, TaskExecutionCont
public static void createProcessLocalPathIfAbsent(TaskExecutionContext taskExecutionContext) throws TaskException {
try {
// local execute path
- String execLocalPath = FileUtils.getProcessExecDir(
- taskExecutionContext.getProjectCode(),
- taskExecutionContext.getProcessDefineCode(),
- taskExecutionContext.getProcessDefineVersion(),
- taskExecutionContext.getProcessInstanceId(),
- taskExecutionContext.getTaskInstanceId());
- taskExecutionContext.setExecutePath(execLocalPath);
- FileUtils.createWorkDirIfAbsent(execLocalPath);
+ // String execLocalPath = FileUtils.getProcessExecDir(
+ // taskExecutionContext.getProjectCode(),
+ // taskExecutionContext.getProcessDefineCode(),
+ // taskExecutionContext.getProcessDefineVersion(),
+ // taskExecutionContext.getProcessInstanceId(),
+ // taskExecutionContext.getTaskInstanbu iceId());
+ // taskExecutionContext.setExecutePath(execLocalPath);
+ FileUtils.createWorkDirIfAbsent(taskExecutionContext.getExecutePath());
Review Comment:
Yes, if it's not yarn job, aop code will not execute. However, all types of tasks will generate execution path (as well as appInfoPath) in master, so i don't get what you mean "impossible to get the execution path", would you like to explain it in detail?
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1293743056
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [24.2% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] sonarcloud[bot] commented on pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1294419061
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [22.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983188738
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -17,12 +17,7 @@
package org.apache.dolphinscheduler.common.utils;
-import static org.apache.dolphinscheduler.common.Constants.DATA_BASEDIR_PATH;
-import static org.apache.dolphinscheduler.common.Constants.FOLDER_SEPARATOR;
-import static org.apache.dolphinscheduler.common.Constants.RESOURCE_VIEW_SUFFIXES;
-import static org.apache.dolphinscheduler.common.Constants.RESOURCE_VIEW_SUFFIXES_DEFAULT_VALUE;
-import static org.apache.dolphinscheduler.common.Constants.UTF_8;
-import static org.apache.dolphinscheduler.common.Constants.YYYYMMDDHHMMSS;
+import static org.apache.dolphinscheduler.common.Constants.*;
Review Comment:
> should we avoid to use `.*`?
Thanks, i'll modify them later!
--
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] hstdream commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
hstdream commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r985069718
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractYarnTask.java:
##########
@@ -107,7 +110,7 @@ public void cancelApplication() throws TaskException {
*/
@Override
public List<String> getApplicationIds() throws TaskException {
- return LogUtils.getAppIdsFromLogFile(taskRequest.getLogPath(), logger);
Review Comment:
It is recommended that the log string be added to the constant.
--
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] fuchanghai commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
fuchanghai commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983289745
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -75,14 +75,14 @@ public static void checkTenantExist(WorkerConfig workerConfig, TaskExecutionCont
public static void createProcessLocalPathIfAbsent(TaskExecutionContext taskExecutionContext) throws TaskException {
try {
// local execute path
- String execLocalPath = FileUtils.getProcessExecDir(
- taskExecutionContext.getProjectCode(),
- taskExecutionContext.getProcessDefineCode(),
- taskExecutionContext.getProcessDefineVersion(),
- taskExecutionContext.getProcessInstanceId(),
- taskExecutionContext.getTaskInstanceId());
- taskExecutionContext.setExecutePath(execLocalPath);
- FileUtils.createWorkDirIfAbsent(execLocalPath);
+ // String execLocalPath = FileUtils.getProcessExecDir(
+ // taskExecutionContext.getProjectCode(),
+ // taskExecutionContext.getProcessDefineCode(),
+ // taskExecutionContext.getProcessDefineVersion(),
+ // taskExecutionContext.getProcessInstanceId(),
+ // taskExecutionContext.getTaskInstanbu iceId());
+ // taskExecutionContext.setExecutePath(execLocalPath);
+ FileUtils.createWorkDirIfAbsent(taskExecutionContext.getExecutePath());
Review Comment:
Your Aspect can only intercept submissions on yarn, right? If the task is not running on yarn, is it impossible to get the execution path?
--
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] fuchanghai commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
fuchanghai commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983298347
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * flag to indicate whether print debug logs
+ */
+ private static final String PARA_NAME_ASPECTJ_DEBUG = "PARA_NAME_ASPECTJ_DEBUG";
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = System.getProperty("user.dir") + "/appInfo.log";
+ debug = Boolean.parseBoolean(System.getenv(PARA_NAME_ASPECTJ_DEBUG));
Review Comment:
if he depends on the common module, it can be solved. I think this variable must be configurable and cannot be configured by modifying the code. if above cann't fits your needs.We should prioritize using the configuration file in this module. Welcome to give your feedback any comments
--
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] fuchanghai commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
fuchanghai commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r983298347
##########
dolphinscheduler-aop/src/main/java/org/apache/dolphinscheduler/aop/YarnClientAspect.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.aop;
+
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.apache.hadoop.yarn.api.records.ApplicationReport;
+import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
+
+import org.aspectj.lang.annotation.AfterReturning;
+import org.aspectj.lang.annotation.Aspect;
+
+@Aspect
+public class YarnClientAspect {
+
+ /**
+ * flag to indicate whether print debug logs
+ */
+ private static final String PARA_NAME_ASPECTJ_DEBUG = "PARA_NAME_ASPECTJ_DEBUG";
+
+ /**
+ * The current application report when application submitted successfully
+ */
+ private ApplicationReport currentApplicationReport = null;
+
+ private String appInfoFilePath;
+ private boolean debug;
+
+ public YarnClientAspect() {
+ appInfoFilePath = System.getProperty("user.dir") + "/appInfo.log";
+ debug = Boolean.parseBoolean(System.getenv(PARA_NAME_ASPECTJ_DEBUG));
Review Comment:
if he depends on the common module, it can be solved. I think this variable must be configurable and cannot be configured by modifying the code
--
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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006532774
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/builder/TaskExecutionContextBuilder.java:
##########
@@ -126,6 +127,24 @@ public TaskExecutionContextBuilder buildProcessDefinitionRelatedInfo(ProcessDefi
return this;
}
+ /**
+ * build execPath related info
+ *
+ * @return TaskExecutionContextBuilder
+ */
+ public TaskExecutionContextBuilder buildExecPathRelatedInfo() {
Review Comment:
> > I think it should setExecutePath and appInfoPath in worker before task running. cc @ruanwenjun
>
> Yes, it should be moved to worker and should be set before yarn task running, I am not sure if other task will need this, you can simply put this before all task running.
>
> @Radeity In fact, the master doesn't know execPath, it is defined in worker, different worker may have different path, master doesn't know the execpath
Oops! I have ignored that different workers may have different `DATA_BASEDIR`. I'll change this part of code, but, i'll still keep `appInfoPath`, is that okay?
--
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] Radeity commented on a diff in pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
Radeity commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006538574
##########
script/env/dolphinscheduler_env.sh:
##########
@@ -33,3 +33,10 @@ export SEATUNNEL_HOME=${SEATUNNEL_HOME:-/opt/soft/seatunnel}
export CHUNJUN_HOME=${CHUNJUN_HOME:-/opt/soft/chunjun}
export PATH=$HADOOP_HOME/bin:$SPARK_HOME/bin:$PYTHON_HOME/bin:$JAVA_HOME/bin:$HIVE_HOME/bin:$FLINK_HOME/bin:$DATAX_HOME/bin:$SEATUNNEL_HOME/bin:$CHUNJUN_HOME/bin:$PATH
+
+# applicationId auto collection related configuration
+export HADOOP_CLASSPATH=`hadoop classpath`:${DOLPHINSCHEDULER_HOME}/tools/libs/*
+export SPARK_DIST_CLASSPATH=$HADOOP_CLASSPATH:$SPARK_DIST_CLASS_PATH
+export HADOOP_CLIENT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$HADOOP_CLIENT_OPTS
+export SPARK_SUBMIT_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$SPARK_SUBMIT_OPTS
+export FLINK_ENV_JAVA_OPTS="-javaagent:${DOLPHINSCHEDULER_HOME}/tools/libs/aspectjweaver-1.9.7.jar":$FLINK_ENV_JAVA_OPTS
Review Comment:
Maybe add some annotation or make statement in docs that only configure that collect appId in aop way need these opts, WDYT?
--
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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1294678069
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [23.2% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] sonarcloud[bot] commented on pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1296681955
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [39 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [45.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [2.1% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] sonarcloud[bot] commented on pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1291666291
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [21.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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] sonarcloud[bot] commented on pull request #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#issuecomment-1291666513
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12197)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12197&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list) [21.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12197&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 #12197: [Improvement][Task] Improved way to collect yarn job's appIds
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12197:
URL: https://github.com/apache/dolphinscheduler/pull/12197#discussion_r1006522382
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/builder/TaskExecutionContextBuilder.java:
##########
@@ -126,6 +127,24 @@ public TaskExecutionContextBuilder buildProcessDefinitionRelatedInfo(ProcessDefi
return this;
}
+ /**
+ * build execPath related info
+ *
+ * @return TaskExecutionContextBuilder
+ */
+ public TaskExecutionContextBuilder buildExecPathRelatedInfo() {
Review Comment:
> I think it should setExecutePath and appInfoPath in worker before task running. cc @ruanwenjun
Yes, it should be moved to worker and should be set before yarn task running, I am not sure if other task will need this,
you can simply put this before all task running.
@Radeity In fact, the master doesn't know execPath, it is defined in worker, different worker may have different path, master doesn't know the execpath
--
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