You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "WHBANG (via GitHub)" <gi...@apache.org> on 2023/02/27 08:26:51 UTC
[GitHub] [incubator-pegasus] WHBANG opened a new pull request, #1370: chore(script): Script migrate_node.sh supports using specified config file when using shell config
WHBANG opened a new pull request, #1370:
URL: https://github.com/apache/incubator-pegasus/pull/1370
### What problem does this PR solve? <!--add issue link with summary if exists-->
https://github.com/apache/incubator-pegasus/issues/1368
- Manual test (add detailed scripts or steps 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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] empiredan merged pull request #1370: chore(script): Script migrate_node.sh supports using specified config file when using shell config
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan merged PR #1370:
URL: https://github.com/apache/incubator-pegasus/pull/1370
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1370: chore(script): Script migrate_node.sh supports using specified config file when using shell config
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1370:
URL: https://github.com/apache/incubator-pegasus/pull/1370#discussion_r1124150592
##########
run.sh:
##########
@@ -1660,6 +1663,11 @@ function run_migrate_node()
CLUSTER="$2"
shift
;;
+ -p|--config)
+ CONFIG="$2"
+ CONFIG_SPECIFIED=1
Review Comment:
`CONFIG_SPECIFIED` can be removed, just judge whether `CONFIG` is empty or not?
##########
run.sh:
##########
@@ -1703,14 +1711,22 @@ function run_migrate_node()
exit 1
fi
- echo "CLUSTER=$CLUSTER"
+ if [ ${CONFIG_SPECIFIED} -eq 0 ]; then
+ echo "CLUSTER=$CLUSTER"
+ else
+ echo "CONFIG=$CONFIG"
+ fi
echo "NODE=$NODE"
echo "APP=$APP"
echo "TYPE=$TYPE"
echo
cd ${ROOT}
echo "------------------------------"
- ./scripts/migrate_node.sh $CLUSTER $NODE "$APP" $TYPE
+ if [ ${CONFIG_SPECIFIED} -eq 0 ]; then
+ ./scripts/migrate_node.sh $CLUSTER $NODE "$APP" $TYPE
+ else
+ ./scripts/migrate_node.sh $CLUSTER $NODE "$APP" $TYPE $CONFIG_SPECIFIED
Review Comment:
Should use `$CONFIG` instead of `$CLUSTER`?
##########
scripts/migrate_node.sh:
##########
@@ -20,37 +20,52 @@ set -e
PID=$$
-if [ $# -ne 4 ]
+if [ $# -ne 4 -a $# -ne 5 ]
then
echo "This tool is for migrating primary replicas out of specified node."
- echo "USAGE: $0 <cluster-meta-list> <migrate-node> <app-name> <run|test>"
- echo " app-name = * means migrate all apps"
+ echo "USAGE1: $0 <cluster-meta-list> <migrate-node> <app-name> <run|test>"
+ echo "USAGE2: $0 <shell-config-path> <migrate-node> <app-name> <run|test> <placeholder>"
+ echo "app-name = * means migrate all apps"
exit 1
fi
pwd="$( cd "$( dirname "$0" )" && pwd )"
shell_dir="$( cd $pwd/.. && pwd )"
cd $shell_dir
-cluster=$1
-node=$2
-app_name=$3
-type=$4
+if [ $# -eq 4 ]; then
+ CONFIG_SPECIFIED=0
+ cluster=$1
+ node=$2
+ app_name=$3
+ type=$4
+else
+ CONFIG_SPECIFIED=1
+ config=$1
+ node=$2
+ app_name=$3
+ type=$4
+fi
Review Comment:
```suggestion
if [ $# -eq 4 ]; then
CONFIG_SPECIFIED=0
cluster=$1
else
CONFIG_SPECIFIED=1
config=$1
fi
node=$2
app_name=$3
type=$4
```
##########
run.sh:
##########
@@ -1703,14 +1711,22 @@ function run_migrate_node()
exit 1
fi
- echo "CLUSTER=$CLUSTER"
+ if [ ${CONFIG_SPECIFIED} -eq 0 ]; then
+ echo "CLUSTER=$CLUSTER"
+ else
+ echo "CONFIG=$CONFIG"
+ fi
echo "NODE=$NODE"
echo "APP=$APP"
echo "TYPE=$TYPE"
echo
cd ${ROOT}
echo "------------------------------"
- ./scripts/migrate_node.sh $CLUSTER $NODE "$APP" $TYPE
+ if [ ${CONFIG_SPECIFIED} -eq 0 ]; then
+ ./scripts/migrate_node.sh $CLUSTER $NODE "$APP" $TYPE
+ else
+ ./scripts/migrate_node.sh $CLUSTER $NODE "$APP" $TYPE $CONFIG_SPECIFIED
Review Comment:
```suggestion
./scripts/migrate_node.sh $CLUSTER $NODE "$APP" $TYPE $CONFIG_SPECIFIED
```
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1370: chore(script): Script migrate_node.sh supports using specified config file when using shell config
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1370:
URL: https://github.com/apache/incubator-pegasus/pull/1370#discussion_r1133933600
##########
scripts/migrate_node.sh:
##########
@@ -20,37 +20,58 @@ set -e
PID=$$
-if [ $# -ne 4 ]
-then
+function usage()
+{
echo "This tool is for migrating primary replicas out of specified node."
- echo "USAGE: $0 <cluster-meta-list> <migrate-node> <app-name> <run|test>"
- echo " app-name = * means migrate all apps"
+ echo
+ echo "USAGE1: $0 <cluster-meta-list> <migrate-node> <app-name> <run|test>"
+ echo "USAGE2: $0 <shell-config-path> <migrate-node> <app-name> <run|test> -f"
+ echo "app-name = * means migrate all apps"
+}
+
+if [ $# -ne 4 -a $# -ne 5 ]
+then
+ usage
exit 1
fi
pwd="$( cd "$( dirname "$0" )" && pwd )"
shell_dir="$( cd $pwd/.. && pwd )"
cd $shell_dir
-cluster=$1
+CONFIG_SPECIFIED=0
+if [ $# -eq 4 ]; then
+ cluster=$1
+elif [ "$5" == "-f" ]; then
+ CONFIG_SPECIFIED=1
+ config=$1
+else
+ usage
+ echo "ERROR: invalid option: $5"
+ exit 1
+fi
node=$2
app_name=$3
type=$4
if [ "$type" != "run" -a "$type" != "test" ]
then
+ usage
echo "ERROR: invalid type: $type"
- echo "USAGE: $0 <cluster-meta-list> <migrate-node> <app-name> <run|test>"
exit 1
fi
echo "UID=$UID"
echo "PID=$PID"
echo
-echo "set_meta_level steady" | ./run.sh shell --cluster $cluster &>/tmp/$UID.$PID.pegasus.set_meta_level
-
-echo ls | ./run.sh shell --cluster $cluster &>/tmp/$UID.$PID.pegasus.ls
+if [ ${CONFIG_SPECIFIED} -eq 0 ]; then
Review Comment:
`CONFIG_SPECIFIED` can be replaced by `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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] empiredan commented on pull request #1370: chore(script): Script migrate_node.sh supports using specified config file when using shell config
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on PR #1370:
URL: https://github.com/apache/incubator-pegasus/pull/1370#issuecomment-1463427034
If it's necessary to be compatible with the previous format of arguments for `migrate_node.sh`, we could add an option `-t` for `migrate_node.sh`:
- `migrate_node.sh -t config <arguments ...>` means the first argument is config, or
- `migrate_node.sh -t cluster <arguments ...>` or just `migrate_node.sh <arguments ...>` (`-t` is omitted) means the first argument is 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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1370: chore(script): Script migrate_node.sh supports using specified config file when using shell config
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1370:
URL: https://github.com/apache/incubator-pegasus/pull/1370#discussion_r1132251837
##########
scripts/migrate_node.sh:
##########
@@ -20,37 +20,49 @@ set -e
PID=$$
-if [ $# -ne 4 ]
+if [ $# -ne 4 -a $# -ne 5 ]
then
echo "This tool is for migrating primary replicas out of specified node."
- echo "USAGE: $0 <cluster-meta-list> <migrate-node> <app-name> <run|test>"
- echo " app-name = * means migrate all apps"
+ echo "USAGE1: $0 <cluster-meta-list> <migrate-node> <app-name> <run|test>"
+ echo "USAGE2: $0 <shell-config-path> <migrate-node> <app-name> <run|test> -f"
+ echo "app-name = * means migrate all apps"
exit 1
fi
pwd="$( cd "$( dirname "$0" )" && pwd )"
shell_dir="$( cd $pwd/.. && pwd )"
cd $shell_dir
-cluster=$1
+CONFIG_SPECIFIED=0
+if [ $# -eq 4 ]; then
+ cluster=$1
+else
+ CONFIG_SPECIFIED=1
Review Comment:
If it is necessary to check whether option is `-f` ?
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org