Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-3517] [dist] Only count active PIDs in start script #1716

Closed
wants to merge 1 commit into from

Conversation

uce
Copy link
Contributor

@uce uce commented Feb 25, 2016

$ bin/start-cluster.sh
Starting cluster.
Starting jobmanager daemon on host pablo.
Starting taskmanager daemon on host pablo.
$ bin/taskmanager.sh start
[INFO] 1 instance(s) of taskmanager are already running on pablo.
Starting taskmanager daemon on host pablo.
$ bin/taskmanager.sh start
[INFO] 2 instance(s) of taskmanager are already running on pablo.
Starting taskmanager daemon on host pablo.
$ bin/taskmanager.sh start
[INFO] 3 instance(s) of taskmanager are already running on pablo.
Starting taskmanager daemon on host pablo.
$ jps
27328 TaskManager
27140 TaskManager
26949 TaskManager
26523 JobManager
26716 TaskManager
$ kill -9 27140
$ bin/taskmanager.sh start
>>> [INFO] 3 instance(s) of taskmanager are already running on pablo <<< Correct now
Starting taskmanager daemon on host pablo.
$ bin/stop-cluster.sh
Stopping taskmanager daemon (pid: 27545) on host pablo.
Stopping jobmanager daemon (pid: 26523) on host pablo.
$ bin/taskmanager.sh stop
Stopping taskmanager daemon (pid: 27328) on host pablo.
$ bin/taskmanager.sh stop
No taskmanager daemon (pid: 27140) is running anymore on pablo.
$ bin/taskmanager.sh stop
Stopping taskmanager daemon (pid: 26949) on host pablo.
$ bin/taskmanager.sh stop
Stopping taskmanager daemon (pid: 26716) on host pablo.
$ bin/taskmanager.sh stop
No taskmanager daemon to stop on host pablo.

We can further improve the stop part by repeatedly the PIDs in the pid file if a value is not matching an active PID.

@mxm
Copy link
Contributor

mxm commented Feb 26, 2016

+1 to merge!

I find "No taskmanager daemon (pid: 27140) is running anymore on pablo." confusing. I think we could change it to something like "TaskManager couldn't be stopped. It has already been shut down." Anyways, it's not part of this issue.

@tillrohrmann
Copy link
Contributor

Code changes look good :-) +1 for merging.

@uce
Copy link
Contributor Author

uce commented Feb 29, 2016

I'm merging this to master and release-1.0.

@asfgit asfgit closed this in e840bbf Feb 29, 2016
subhankarb pushed a commit to subhankarb/flink that referenced this pull request Mar 17, 2016
@uce uce deleted the 3517-scripts branch April 20, 2016 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants