Skip to content

[fix-11176][Bug] [script] dolphinscheduler-daemon.sh Status error #11176#11182

Closed
TangTangGrit wants to merge 9 commits intoapache:devfrom
TangTangGrit:dev
Closed

[fix-11176][Bug] [script] dolphinscheduler-daemon.sh Status error #11176#11182
TangTangGrit wants to merge 9 commits intoapache:devfrom
TangTangGrit:dev

Conversation

@TangTangGrit
Copy link

Purpose of the pull request

close #11176

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@SbloodyS SbloodyS added the first time contributor First-time contributor label Jul 28, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2022

Codecov Report

Merging #11182 (b669243) into dev (a52d1b4) will decrease coverage by 0.06%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                dev   #11182      +/-   ##
============================================
- Coverage     40.23%   40.16%   -0.07%     
- Complexity     4924     4947      +23     
============================================
  Files           982      988       +6     
  Lines         37567    37688     +121     
  Branches       4129     4135       +6     
============================================
+ Hits          15116    15139      +23     
- Misses        20915    21007      +92     
- Partials       1536     1542       +6     
Impacted Files Coverage Δ
...erver/master/processor/queue/TaskEventService.java 69.64% <0.00%> (-10.72%) ⬇️
...e/dolphinscheduler/remote/NettyRemotingClient.java 50.00% <0.00%> (-2.78%) ⬇️
...pache/dolphinscheduler/common/utils/DateUtils.java 74.83% <0.00%> (-0.17%) ⬇️
.../dolphinscheduler/plugin/task/datax/DataxTask.java 0.00% <0.00%> (ø)
...ler/plugin/task/api/loop/BaseLoopTaskExecutor.java 0.00% <0.00%> (ø)
...loop/template/http/HttpLoopTaskInstanceStatus.java 0.00% <0.00%> (ø)
...eduler/plugin/task/chunjun/ChunJunTaskChannel.java 0.00% <0.00%> (ø)
...plugin/task/chunjun/ChunJunTaskChannelFactory.java 0.00% <0.00%> (ø)
...ugin/task/chunjun/ChunJunTaskExecutionContext.java 0.00% <0.00%> (ø)
...cheduler/plugin/task/chunjun/ChunJunConstants.java 0.00% <0.00%> (ø)
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@github-actions github-actions bot removed the UI ui and front end related label Jul 28, 2022
@zhongjiajie zhongjiajie added this to the 3.0.0-release milestone Jul 29, 2022
@zhongjiajie
Copy link
Member

I wonder, can we directly check the pid file of each service?

@SbloodyS
Copy link
Member

I wonder, can we directly check the pid file of each service?

That sounds more reasonable to me.

@TangTangGrit
Copy link
Author

TangTangGrit commented Jul 29, 2022

I wonder, can we directly check the pid file of each service?

I have checked. There is no individual service PID

@SbloodyS
Copy link
Member

I wonder, can we directly check the pid file of each service?

I have checked. There is no individual service PID

We do have it. You can take a look at

pid=$DOLPHINSCHEDULER_HOME/$command/pid
cd $DOLPHINSCHEDULER_HOME/$command
if [ "$command" = "api-server" ]; then
log=$DOLPHINSCHEDULER_HOME/api-server/logs/$command-$HOSTNAME.out
elif [ "$command" = "master-server" ]; then
log=$DOLPHINSCHEDULER_HOME/master-server/logs/$command-$HOSTNAME.out
elif [ "$command" = "worker-server" ]; then
log=$DOLPHINSCHEDULER_HOME/worker-server/logs/$command-$HOSTNAME.out
elif [ "$command" = "alert-server" ]; then
log=$DOLPHINSCHEDULER_HOME/alert-server/logs/$command-$HOSTNAME.out
elif [ "$command" = "standalone-server" ]; then
log=$DOLPHINSCHEDULER_HOME/standalone-server/logs/$command-$HOSTNAME.out
else
echo "Error: No command named '$command' was found."
exit 1
fi
case $startStop in
(start)
echo starting $command, logging to $DOLPHINSCHEDULER_LOG_DIR
overwrite_server_env "${command}"
nohup /bin/bash "$DOLPHINSCHEDULER_HOME/$command/bin/start.sh" > $log 2>&1 &
echo $! > $pid

@TangTangGrit
Copy link
Author

I wonder, can we directly check the pid file of each service?

I have checked. There is no individual service PID

We do have it. You can take a look at

pid=$DOLPHINSCHEDULER_HOME/$command/pid
cd $DOLPHINSCHEDULER_HOME/$command
if [ "$command" = "api-server" ]; then
log=$DOLPHINSCHEDULER_HOME/api-server/logs/$command-$HOSTNAME.out
elif [ "$command" = "master-server" ]; then
log=$DOLPHINSCHEDULER_HOME/master-server/logs/$command-$HOSTNAME.out
elif [ "$command" = "worker-server" ]; then
log=$DOLPHINSCHEDULER_HOME/worker-server/logs/$command-$HOSTNAME.out
elif [ "$command" = "alert-server" ]; then
log=$DOLPHINSCHEDULER_HOME/alert-server/logs/$command-$HOSTNAME.out
elif [ "$command" = "standalone-server" ]; then
log=$DOLPHINSCHEDULER_HOME/standalone-server/logs/$command-$HOSTNAME.out
else
echo "Error: No command named '$command' was found."
exit 1
fi
case $startStop in
(start)
echo starting $command, logging to $DOLPHINSCHEDULER_LOG_DIR
overwrite_server_env "${command}"
nohup /bin/bash "$DOLPHINSCHEDULER_HOME/$command/bin/start.sh" > $log 2>&1 &
echo $! > $pid

I'm based on advice. The code has been reworked. But what am I supposed to do now

@SbloodyS
Copy link
Member

What i mean is we can get the pid of each service through the pid file to verify whether the process is alive. @TangTangGrit

@TangTangGrit
Copy link
Author

What i mean is we can get the pid of each service through the pid file to verify whether the process is alive. @TangTangGrit

Yes, that's what I did

@SbloodyS
Copy link
Member

What i mean is we can get the pid of each service through the pid file to verify whether the process is alive. @TangTangGrit

Yes, that's what I did

I see. Sorry, I was careless and didn't take a close look at your modification.

SbloodyS
SbloodyS previously approved these changes Jul 29, 2022
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

fix the alert-server and standalone-server class
SbloodyS
SbloodyS previously approved these changes Jul 29, 2022
Comment on lines +111 to +123
state="STOP"
state="[ \033[1;31m $state \033[0m ]"
if [ -f $pid ]; then
TARGET_PID=`cat $pid`
serverCount=`ps -p $TARGET_PID | wc -l`
if [[ $serverCount -gt 1 ]];then
state="RUNNING"
state="[ \033[1;32m $state \033[0m ]"
fi
echo -e "$command $state"
fi
;;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not do this. The previous version is enough. Please rollback it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the,This is verified by obtaining the PID of each service from the PID file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as you said

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. But the old method also realizes this function. We should try to reduce large-scale modifications to ensure stability.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SbloodyS I think this is better than the old, more beautiful ^_^

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is want to used the pid as filter rather than use the process name.
If the pid file has been deleted, this method may return an incorrect status.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

50.0% 50.0% Coverage
0.0% 0.0% Duplication

@zhongjiajie zhongjiajie modified the milestones: 3.0.0, 3.0.1 Aug 5, 2022
@apache apache deleted a comment from luojieio Aug 17, 2022
;;

state="STOP"
state="[ \033[1;31m $state \033[0m ]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you remove the comment # font color - red?

@caishunfeng caishunfeng added the bug Something isn't working label Sep 16, 2022
@zhuangchong zhuangchong modified the milestones: 3.0.1, 3.0.2 Sep 18, 2022
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Oct 24, 2023
@github-actions
Copy link

This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.

@github-actions github-actions bot closed this Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend bug Something isn't working first time contributor First-time contributor priority:high Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [script] dolphinscheduler-daemon.sh Status error

8 participants