You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by NicoK <gi...@git.apache.org> on 2017/02/13 14:47:41 UTC
[GitHub] flink pull request #3298: [FLINK-5672] add special cases for a local setup i...
GitHub user NicoK opened a pull request:
https://github.com/apache/flink/pull/3298
[FLINK-5672] add special cases for a local setup in cluster start/stop scripts
With this PR, if all slaves refer to `"localhost"` we run the daemons from the script itself instead of using ssh which may not be available. This can be seen as a first step in removing the local mode alltogether.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/NicoK/flink flink-5672
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3298.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3298
----
commit 73ba199f9c19a5f6a38dbd6e401d372df7bffd4b
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2017-02-13T14:33:23Z
[FLINK-5672] add special cases for a local setup in cluster start/stop scripts
This way, if all slaves refer to "localhost", we do not require ssh at all.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3298: [FLINK-5672] add special cases for a local setup i...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/3298#discussion_r100978413
--- Diff: flink-dist/src/main/flink-bin/bin/stop-cluster.sh ---
@@ -25,14 +25,30 @@ bin=`cd "$bin"; pwd`
# Stop TaskManager instance(s) using pdsh (Parallel Distributed Shell) when available
readSlaves
-command -v pdsh >/dev/null 2>&1
-if [[ $? -ne 0 ]]; then
+# all-local setup?
+all_localhost=1
+for slave in ${SLAVES[@]}; do
+ if [[ "$slave" != "localhost" ]]; then
+ all_localhost=0
+ break
+ fi
+done
+
+if [[ ${all_localhost} -eq 1 ]]; then
+ # all-local setup
for slave in ${SLAVES[@]}; do
- ssh -n $FLINK_SSH_OPTS $slave -- "nohup /bin/bash -l \"${FLINK_BIN_DIR}/taskmanager.sh\" stop &"
+ "${FLINK_BIN_DIR}"/taskmanager.sh stop
done
else
- PDSH_SSH_ARGS="" PDSH_SSH_ARGS_APPEND=$FLINK_SSH_OPTS pdsh -w $(IFS=, ; echo "${SLAVES[*]}") \
- "nohup /bin/bash -l \"${FLINK_BIN_DIR}/taskmanager.sh\" stop"
+ command -v pdsh >/dev/null 2>&1
+ if [[ $? -ne 0 ]]; then
+ for slave in ${SLAVES[@]}; do
+ ssh -n $FLINK_SSH_OPTS $slave -- "nohup /bin/bash -l \"${FLINK_BIN_DIR}/taskmanager.sh\" stop &"
+ done
+ else
+ PDSH_SSH_ARGS="" PDSH_SSH_ARGS_APPEND=$FLINK_SSH_OPTS pdsh -w $(IFS=, ; echo "${SLAVES[*]}") \
+ "nohup /bin/bash -l \"${FLINK_BIN_DIR}/taskmanager.sh\" stop"
--- End diff --
Correct me if I'm wrong, but maybe this part of the code only differs in the "start" or "stop" string.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3298: [FLINK-5672] add special cases for a local setup i...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/3298
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3298: [FLINK-5672] add special cases for a local setup i...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/3298#discussion_r100978305
--- Diff: flink-dist/src/main/flink-bin/bin/stop-cluster.sh ---
@@ -25,14 +25,30 @@ bin=`cd "$bin"; pwd`
# Stop TaskManager instance(s) using pdsh (Parallel Distributed Shell) when available
readSlaves
-command -v pdsh >/dev/null 2>&1
-if [[ $? -ne 0 ]]; then
+# all-local setup?
+all_localhost=1
+for slave in ${SLAVES[@]}; do
+ if [[ "$slave" != "localhost" ]]; then
+ all_localhost=0
+ break
+ fi
+done
--- End diff --
I think we should put the all localhost check into a method (maybe into the config.sh) to avoid duplicate code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3298: [FLINK-5672] add special cases for a local setup i...
Posted by NicoK <gi...@git.apache.org>.
Github user NicoK commented on a diff in the pull request:
https://github.com/apache/flink/pull/3298#discussion_r101008339
--- Diff: flink-dist/src/main/flink-bin/bin/stop-cluster.sh ---
@@ -25,14 +25,30 @@ bin=`cd "$bin"; pwd`
# Stop TaskManager instance(s) using pdsh (Parallel Distributed Shell) when available
readSlaves
-command -v pdsh >/dev/null 2>&1
-if [[ $? -ne 0 ]]; then
+# all-local setup?
+all_localhost=1
+for slave in ${SLAVES[@]}; do
+ if [[ "$slave" != "localhost" ]]; then
+ all_localhost=0
+ break
+ fi
+done
+
+if [[ ${all_localhost} -eq 1 ]]; then
+ # all-local setup
for slave in ${SLAVES[@]}; do
- ssh -n $FLINK_SSH_OPTS $slave -- "nohup /bin/bash -l \"${FLINK_BIN_DIR}/taskmanager.sh\" stop &"
+ "${FLINK_BIN_DIR}"/taskmanager.sh stop
done
else
- PDSH_SSH_ARGS="" PDSH_SSH_ARGS_APPEND=$FLINK_SSH_OPTS pdsh -w $(IFS=, ; echo "${SLAVES[*]}") \
- "nohup /bin/bash -l \"${FLINK_BIN_DIR}/taskmanager.sh\" stop"
+ command -v pdsh >/dev/null 2>&1
+ if [[ $? -ne 0 ]]; then
+ for slave in ${SLAVES[@]}; do
+ ssh -n $FLINK_SSH_OPTS $slave -- "nohup /bin/bash -l \"${FLINK_BIN_DIR}/taskmanager.sh\" stop &"
+ done
+ else
+ PDSH_SSH_ARGS="" PDSH_SSH_ARGS_APPEND=$FLINK_SSH_OPTS pdsh -w $(IFS=, ; echo "${SLAVES[*]}") \
+ "nohup /bin/bash -l \"${FLINK_BIN_DIR}/taskmanager.sh\" stop"
--- End diff --
you're right but it's probably more readable to keep the commands in there instead of sharing them, also it has been that way before ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3298: [FLINK-5672] add special cases for a local setup in clust...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3298
Looks good, nice step to get away from the `start-local.sh` script.
Merging this...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3298: [FLINK-5672] add special cases for a local setup i...
Posted by NicoK <gi...@git.apache.org>.
Github user NicoK commented on a diff in the pull request:
https://github.com/apache/flink/pull/3298#discussion_r101008195
--- Diff: flink-dist/src/main/flink-bin/bin/stop-cluster.sh ---
@@ -25,14 +25,30 @@ bin=`cd "$bin"; pwd`
# Stop TaskManager instance(s) using pdsh (Parallel Distributed Shell) when available
readSlaves
-command -v pdsh >/dev/null 2>&1
-if [[ $? -ne 0 ]]; then
+# all-local setup?
+all_localhost=1
+for slave in ${SLAVES[@]}; do
+ if [[ "$slave" != "localhost" ]]; then
+ all_localhost=0
+ break
+ fi
+done
--- End diff --
makes sense - also since there we already go through the slaves once so we can do this check on the fly
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---