You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/06/20 12:53:58 UTC
[GitHub] [dolphinscheduler] ruanwenjun opened a new pull request, #10516: Optimize master log, add workflow instance id and task instance id in log
ruanwenjun opened a new pull request, #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516
## Purpose of the pull request
close #10496
Add log prefix help for troubleshuting.
--
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 #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#discussion_r902165409
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/processor/queue/StateEventResponseService.java:
##########
@@ -109,18 +113,21 @@ protected StateEventResponseWorker() {
@Override
public void run() {
-
+ logger.info("State event loop service started");
while (Stopper.isRunning()) {
try {
// if not task , blocking here
StateEvent stateEvent = eventQueue.take();
+ MDC.put(Constants.WORKFLOW_INFO_MDC_KEY, String.format(Constants.WORKFLOW_TASK_HEADER_FORMAT, stateEvent.getProcessInstanceId(), stateEvent.getTaskInstanceId()));
persist(stateEvent);
+ MDC.remove(Constants.WORKFLOW_INFO_MDC_KEY);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun merged pull request #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
ruanwenjun merged PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516
--
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 #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#discussion_r902215873
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java:
##########
@@ -620,28 +623,33 @@ private Constants() {
*/
public static final String LOGIN_USER_KEY_TAB_PATH = "login.user.keytab.path";
+ public static final String WORKFLOW_INFO_MDC_KEY = "workflowInfo";
+ public static final String WORKFLOW_LOG_PREFIX_FORMAT = "[WorkflowInstance-%s]";
Review Comment:
Make sense to me.
--
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 #10516: Optimize master log, add workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#issuecomment-1160430628
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10516?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 [#10516](https://codecov.io/gh/apache/dolphinscheduler/pull/10516?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fd614aa) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/79ce5d78cd22e5d548bd3285e169ba51b1a83d26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (79ce5d7) will **decrease** coverage by `0.09%`.
> The diff coverage is `7.28%`.
```diff
@@ Coverage Diff @@
## dev #10516 +/- ##
============================================
- Coverage 40.87% 40.78% -0.10%
+ Complexity 4852 4847 -5
============================================
Files 886 888 +2
Lines 36032 36101 +69
Branches 3998 3993 -5
============================================
- Hits 14729 14723 -6
- Misses 19847 19919 +72
- Partials 1456 1459 +3
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/10516?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/dolphinscheduler/common/thread/Stopper.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/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-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3RocmVhZC9TdG9wcGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...he/dolphinscheduler/common/thread/ThreadUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/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-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3RocmVhZC9UaHJlYWRVdGlscy5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...e/dolphinscheduler/server/master/MasterServer.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9NYXN0ZXJTZXJ2ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...r/server/master/processor/StateEventProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvU3RhdGVFdmVudFByb2Nlc3Nvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...er/server/master/processor/TaskEventProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvVGFza0V2ZW50UHJvY2Vzc29yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...master/processor/TaskExecuteResponseProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvVGFza0V4ZWN1dGVSZXNwb25zZVByb2Nlc3Nvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...r/server/master/processor/TaskRecallProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvVGFza1JlY2FsbFByb2Nlc3Nvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ter/processor/queue/StateEventResponseService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvcXVldWUvU3RhdGVFdmVudFJlc3BvbnNlU2VydmljZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...uler/server/master/runner/EventExecuteService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvRXZlbnRFeGVjdXRlU2VydmljZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...er/server/master/runner/FailoverExecuteThread.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvRmFpbG92ZXJFeGVjdXRlVGhyZWFkLmphdmE=) | `0.00% <0.00%> (ø)` | |
| ... and [30 more](https://codecov.io/gh/apache/dolphinscheduler/pull/10516/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10516?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10516?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [79ce5d7...fd614aa](https://codecov.io/gh/apache/dolphinscheduler/pull/10516?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#issuecomment-1161340773
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10516)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list) [12.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_duplicated_lines_density&view=list) [1.7% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&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 #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#discussion_r902167032
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java:
##########
@@ -620,28 +623,33 @@ private Constants() {
*/
public static final String LOGIN_USER_KEY_TAB_PATH = "login.user.keytab.path";
+ public static final String WORKFLOW_INFO_MDC_KEY = "workflowInfo";
+ public static final String WORKFLOW_LOG_PREFIX_FORMAT = "[WorkflowInstance-%s]";
Review Comment:
Good idea, I add `workflowInstanceId` and `taskInstanceId` in MDC. Then user can set the pattern by themselves.
But some thread may does have these two value in MDC, so the log may look a little strange.
--
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 #10516: Optimize master log, add workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#issuecomment-1160437794
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10516)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list) [7.9% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_duplicated_lines_density&view=list) [2.3% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&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 #10516: Optimize master log, add workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#discussion_r901737687
##########
dolphinscheduler-alert/dolphinscheduler-alert-server/src/main/java/org/apache/dolphinscheduler/alert/AlertServer.java:
##########
@@ -88,24 +91,23 @@ public void close() {
public void destroy(String cause) {
try {
+ // set stop signal is true
// execute only once
- if (Stopper.isStopped()) {
+ if (!Stopper.stop()) {
+ logger.warn("AlterServer is already stopped");
Review Comment:
I have changed this method to `compareAndSwap`, so the stop method will only be executed successful only once.
```java
/**
* Stop the server
*
* @return True, if the server stopped success. False, if the server is already stopped.
*/
public static boolean stop() {
return stoppedSignal.compareAndSet(false, true);
}
```
##########
dolphinscheduler-alert/dolphinscheduler-alert-server/src/main/java/org/apache/dolphinscheduler/alert/AlertServer.java:
##########
@@ -67,7 +70,7 @@ public static void main(String[] args) {
@EventListener
public void run(ApplicationReadyEvent readyEvent) {
- logger.info("alert server starting...");
+ logger.info("Alert server is ready ...");
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on pull request #10516: Optimize master log, add workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#issuecomment-1160523594
> Just take a quick look, and did you check whether MDC applies in this case? I think we can leverage MDC to uniformly add workflow id / instance id / task id / task instance id , etc. in our logs without too many efforts, also we won't miss these when adding new logs in the future.
>
> MDC: https://logback.qos.ch/manual/mdc.html
Thanks for your wonderful suggestion, MDC can work well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10516: Optimize master log, add workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#discussion_r901639628
##########
dolphinscheduler-alert/dolphinscheduler-alert-server/src/main/java/org/apache/dolphinscheduler/alert/AlertServer.java:
##########
@@ -67,7 +70,7 @@ public static void main(String[] args) {
@EventListener
public void run(ApplicationReadyEvent readyEvent) {
- logger.info("alert server starting...");
+ logger.info("Alert server is ready ...");
Review Comment:
This is kinda misleading, it's not ready yet here, it's only ready after `alertSenderService.start();`
##########
dolphinscheduler-alert/dolphinscheduler-alert-server/src/main/java/org/apache/dolphinscheduler/alert/AlertServer.java:
##########
@@ -88,24 +91,23 @@ public void close() {
public void destroy(String cause) {
try {
+ // set stop signal is true
// execute only once
- if (Stopper.isStopped()) {
+ if (!Stopper.stop()) {
+ logger.warn("AlterServer is already stopped");
Review Comment:
If `!Stopper.stop()` why is it already stopped? Shouldn't `!Stopper.stop()` means it's not stopped yet?
--
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 #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#issuecomment-1161154575
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10516)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list) [7.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_duplicated_lines_density&view=list) [1.9% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&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 #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#issuecomment-1161283932
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10516)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list) [8.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_duplicated_lines_density&view=list) [1.5% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&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] kezhenxu94 commented on a diff in pull request #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#discussion_r904444493
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/processor/TaskKillResponseProcessor.java:
##########
@@ -51,7 +51,8 @@ public void process(Channel channel, Command command) {
Preconditions.checkArgument(CommandType.TASK_KILL_RESPONSE == command.getType(), String.format("invalid command type : %s", command.getType()));
TaskKillResponseCommand responseCommand = JSONUtils.parseObject(command.getBody(), TaskKillResponseCommand.class);
- logger.info("received task kill response command : {}", responseCommand);
+ logger.info("[TaskInstance-{}] Received task kill response command : {}",
Review Comment:
Looks like this class/method does nothing but just log the message?
--
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 #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#discussion_r904489963
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/processor/TaskKillResponseProcessor.java:
##########
@@ -51,7 +51,8 @@ public void process(Channel channel, Command command) {
Preconditions.checkArgument(CommandType.TASK_KILL_RESPONSE == command.getType(), String.format("invalid command type : %s", command.getType()));
TaskKillResponseCommand responseCommand = JSONUtils.parseObject(command.getBody(), TaskKillResponseCommand.class);
- logger.info("received task kill response command : {}", responseCommand);
+ logger.info("[TaskInstance-{}] Received task kill response command : {}",
Review Comment:
Yes, this might be a bug, we may need to judge the kill status and update the database.
One situation is that we believe it kill success, but it still running.
cc @caishunfeng
--
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 #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#issuecomment-1162904039
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10516)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list) [7.4% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_duplicated_lines_density&view=list) [1.8% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&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 #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#discussion_r902167032
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java:
##########
@@ -620,28 +623,33 @@ private Constants() {
*/
public static final String LOGIN_USER_KEY_TAB_PATH = "login.user.keytab.path";
+ public static final String WORKFLOW_INFO_MDC_KEY = "workflowInfo";
+ public static final String WORKFLOW_LOG_PREFIX_FORMAT = "[WorkflowInstance-%s]";
Review Comment:
Good idea, I add `workflowInstanceId` and `taskInstanceId` in MDC. Then user can set the pattern by themselves.
But some thread may does have these two value in MDC, so the log may look a little strange.
```java
[INFO] 2022-06-21 13:02:08.752 +0800 org.apache.dolphinscheduler.server.master.service.FailoverService:[91] - [WorkflowInstance-0][TaskInstance-0] - Master failover service 192.168.1.3:5678 begin to failover hosts:[192.168.1.3:5678]
[INFO] 2022-06-21 13:02:08.873 +0800 org.apache.dolphinscheduler.server.master.service.FailoverService:[143] - [WorkflowInstance-0][TaskInstance-0] - start master[192.168.1.3:5678] failover, need to failover process list size:1
[INFO] 2022-06-21 13:02:08.880 +0800 org.apache.dolphinscheduler.server.master.service.FailoverService:[156] - [WorkflowInstance-0][TaskInstance-0] - failover task instance id: 2151, process instance id: 2008
[INFO] 2022-06-21 13:02:08.880 +0800 org.apache.dolphinscheduler.server.master.service.FailoverService:[173] - [WorkflowInstance-0][TaskInstance-0] - master[192.168.1.3:5678] failover end, useTime:13ms
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#discussion_r902211843
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java:
##########
@@ -620,28 +623,33 @@ private Constants() {
*/
public static final String LOGIN_USER_KEY_TAB_PATH = "login.user.keytab.path";
+ public static final String WORKFLOW_INFO_MDC_KEY = "workflowInfo";
+ public static final String WORKFLOW_LOG_PREFIX_FORMAT = "[WorkflowInstance-%s]";
Review Comment:
> Good idea, I add `workflowInstanceId` and `taskInstanceId` in MDC. Then user can set the pattern by themselves. But some thread may does have these two value in MDC, so the log may look a little strange.
>
> ```java
> [INFO] 2022-06-21 13:02:08.752 +0800 org.apache.dolphinscheduler.server.master.service.FailoverService:[91] - [WorkflowInstance-0][TaskInstance-0] - Master failover service 192.168.1.3:5678 begin to failover hosts:[192.168.1.3:5678]
> [INFO] 2022-06-21 13:02:08.873 +0800 org.apache.dolphinscheduler.server.master.service.FailoverService:[143] - [WorkflowInstance-0][TaskInstance-0] - start master[192.168.1.3:5678] failover, need to failover process list size:1
> [INFO] 2022-06-21 13:02:08.880 +0800 org.apache.dolphinscheduler.server.master.service.FailoverService:[156] - [WorkflowInstance-0][TaskInstance-0] - failover task instance id: 2151, process instance id: 2008
> [INFO] 2022-06-21 13:02:08.880 +0800 org.apache.dolphinscheduler.server.master.service.FailoverService:[173] - [WorkflowInstance-0][TaskInstance-0] - master[192.168.1.3:5678] failover end, useTime:13ms
> ```
That's not a bit deal in my opinion, when setting log patterns, there are some fields without values but we still want the placeholder there so when users want to parse the logs they can have a uniform pattern (and filter some logs for example by `> 0`) instead of configuring many patterns for different logs
--
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 #10516: Optimize master log, add workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#issuecomment-1160561221
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10516)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10516&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list) [7.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&metric=new_duplicated_lines_density&view=list) [2.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10516&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] kezhenxu94 commented on a diff in pull request #10516: Optimize master log, use MDC to inject workflow instance id and task instance id in log
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #10516:
URL: https://github.com/apache/dolphinscheduler/pull/10516#discussion_r902100604
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/processor/queue/StateEventResponseService.java:
##########
@@ -109,18 +113,21 @@ protected StateEventResponseWorker() {
@Override
public void run() {
-
+ logger.info("State event loop service started");
while (Stopper.isRunning()) {
try {
// if not task , blocking here
StateEvent stateEvent = eventQueue.take();
+ MDC.put(Constants.WORKFLOW_INFO_MDC_KEY, String.format(Constants.WORKFLOW_TASK_HEADER_FORMAT, stateEvent.getProcessInstanceId(), stateEvent.getTaskInstanceId()));
persist(stateEvent);
+ MDC.remove(Constants.WORKFLOW_INFO_MDC_KEY);
Review Comment:
Move this to `finally` block?
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java:
##########
@@ -620,28 +623,33 @@ private Constants() {
*/
public static final String LOGIN_USER_KEY_TAB_PATH = "login.user.keytab.path";
+ public static final String WORKFLOW_INFO_MDC_KEY = "workflowInfo";
+ public static final String WORKFLOW_LOG_PREFIX_FORMAT = "[WorkflowInstance-%s]";
Review Comment:
Usually we just set the workflow instance id / task instance id (without `[]`, `WorkflowInstanc-`, only the id like `123`), and let the users have the ability to configure another log pattern, we can move the pattern `[WorkflowInstance-%s]` to the `logback.xml`, (`[WorkflowInstance-%X{workflowId}]`, `[WorkflowInstance-%X{workflowInstanceId}]`), so when users want to collect the logs into their log system they can configure a more parsable log pattern.
--
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