Add process subcommands for multiple nodes #17887

twalluxio · 2023-08-04T01:33:07Z

Add process commands with multiple nodes to golang CLI as part of #17522

bin/alluxio-start.sh masters -> bin/cli.sh process start masters
bin/alluxio-start.sh job_masters -> bin/cli.sh process start job_masters
bin/alluxio-start.sh workers -> bin/cli.sh process start workers
bin/alluxio-start.sh job_workers -> bin/cli.sh process start job_workers
bin/alluxio-start.sh proxies -> bin/cli.sh process start proxies
bin/alluxio-start.sh all -> bin/cli.sh process start all
bin/alluxio-start.sh local -> bin/cli.sh process start local

bin/alluxio-stop.sh masters -> bin/cli.sh process stop masters
bin/alluxio-stop.sh job_masters -> bin/cli.sh process stop job_masters
bin/alluxio-stop.sh workers -> bin/cli.sh process stop workers
bin/alluxio-stop.sh job_workers -> bin/cli.sh process stop job_workers
bin/alluxio-stop.sh proxies -> bin/cli.sh process stop proxies
bin/alluxio-stop.sh all -> bin/cli.sh process stop all
bin/alluxio-stop.sh local -> bin/cli.sh process stop local

For command process start/stop on multiple nodes, using crypto's ssh package to create an SSH session, connect to masters, workers or all nodes, then send according subcommand on single nodes to these nodes.

alluxio-bot · 2023-08-04T06:49:08Z

Automated checks report:

PR title follows the conventions: FAIL
- The title of the PR does not pass all the checks. Please fix the following issues:
  - Supported title prefixes are: [WIP], [SMALLFIX], [DOCFIX]
Commits associated with Github account: PASS

Some checks failed. Please fix the reported issues and reply 'alluxio-bot, check this please' to re-run checks.

alluxio-bot · 2023-08-04T07:04:48Z

Automated checks report:

PR title follows the conventions: PASS
Commits associated with Github account: PASS

All checks passed!

cli/src/alluxio.org/cli/processes/common.go

cli/src/alluxio.org/cli/processes/job_masters.go

cli/src/alluxio.org/cli/processes/job_workers.go

cli/src/alluxio.org/cli/processes/all.go

Xenorith · 2023-08-04T18:17:03Z

cli/src/alluxio.org/cli/processes/all.go

+		subProcess := subProcess
+		err := subProcess.Start(cmd)
+		if err != nil {
+			log.Logger.Errorf("Error: %s", err)


in the current behavior, if a group of processes fails to start (ex. one or more masters do not successfully start), does it continue to try to start the remaining processes?

In this behavior, if one behavior fail, it will continue to start the remaining processes, but give error messages. This is the same to the alluxio-start.sh version. If it needs immediate termination, I can change Errorf to Fatalf, which leads to immediate termination after single process failure.

cli/src/alluxio.org/cli/processes/all.go

cli/src/alluxio.org/cli/processes/common.go

cli/src/alluxio.org/cli/processes/job_masters.go

cli/src/alluxio.org/cli/processes/job_workers.go

cli/src/alluxio.org/cli/processes/local.go

Xenorith

the hard part to get it to work functionally is done, but there is still clean up work to make sure the code to scalable and extensible for others to understand and improve upon.

… to common.go

cli/src/alluxio.org/cli/processes/all.go

Xenorith · 2023-08-07T04:56:31Z

cli/src/alluxio.org/cli/processes/common.go

-	if err != nil {
-		log.Logger.Errorf("Error reading worker hostnames at %s", mastersDir)
-		return nil, err
+var cliPath = filepath.Join(env.Env.EnvVar.GetString(env.ConfAlluxioHome.EnvVar), "bin", "alluxio")


cliPath is dependent on env.ConfAlluxioHome.EnvVar to be properly set, but the environment variables are set in the beginning of the command via

rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { if flagDebugLog { log.Logger.SetLevel(logrus.DebugLevel) } if err := env.InitAlluxioEnv(filepath.Clean(flagRootPath), flagIsDeployed); err != nil { return stacktrace.Propagate(err, "error defining alluxio environment") } return nil }

in launch.go

so this will currently return an empty string, but it will happen to work if you are running the cli from the home directory. so you should store bin/alluxio as a constant string, but each time you want to get the cli path, you'll have to define filepath.Join(env.Env.EnvVar.GetString(env.ConfAlluxioHome.EnvVar), binAlluxioPath)

cli/src/alluxio.org/cli/processes/common.go

Kai-Zhang · 2023-08-07T07:53:05Z

cli/src/alluxio.org/cli/processes/common.go

+	// get list of masters and workers
+	var nodes []string
+	if onMasters {
+		if len(masterList) == 0 {


another way is to move the hostname handling outside the runCommand:

func (p *FooProcess) Start(cmd *env.StartProcessCommand) error { command := addStartFlags("process start foo", cmd) // For master-only situations nodes := getNodes(filepath.Join(env.Env.EnvVar.GetString(env.ConfAlluxioConfDir.EnvVar), "masters")) // For masters & workers nodes = append(nodes, getNodes(filepath.Join(env.Env.EnvVar.GetString(env.ConfAlluxioConfDir.EnvVar), "workers"))...) runCommand(command, nodes) }

in this case, it is no need to place the file parsing in a multi-threading function since it's not very time-consuming. so we can parse the file, then call the function with the whole node list.
avoid using the bool as a switch of two different functions and use the actual different parameters directly.

Kai-Zhang · 2023-08-07T08:03:16Z

cli/src/alluxio.org/cli/processes/common.go

+
+	if len(errors) != 0 {
+		log.Logger.Warningf("Run command %s failed, number of failures: %v", command, len(errors))
+		return stacktrace.Propagate(errors[0], "First error: ")


i'm not sure if we need to log every error here or dump it to some file for debugging. but i'm agree with the current structure: handling the error list, and return a digest error for the caller.

cli/src/alluxio.org/cli/processes/common.go

Xenorith

this looks a lot better and easier to understand when compared to the initial review. a few more minor comments but otherwise it looks good

cli/src/alluxio.org/cli/env/env.go

cli/src/alluxio.org/cli/processes/common.go

cli/src/alluxio.org/cli/processes/job_masters.go

cli/src/alluxio.org/cli/processes/common.go

Xenorith

for all the new strings.Join(), join with space " " instead of empty string ""?

Xenorith · 2023-08-08T06:21:35Z

alluxio-bot, merge this please

alluxio-bot · 2023-08-08T06:21:37Z

merge failed:
Merge refused because pull request does not have label start with type-

Kai-Zhang · 2023-08-08T07:02:41Z

alluxio-bot, merge this please

Add `process` commands with multiple nodes to golang CLI as part of #17522 `bin/alluxio-start.sh masters` -> `bin/cli.sh process start masters` `bin/alluxio-start.sh job_masters` -> `bin/cli.sh process start job_masters` `bin/alluxio-start.sh workers` -> `bin/cli.sh process start workers` `bin/alluxio-start.sh job_workers` -> `bin/cli.sh process start job_workers` `bin/alluxio-start.sh proxies` -> `bin/cli.sh process start proxies` `bin/alluxio-start.sh all` -> `bin/cli.sh process start all` `bin/alluxio-start.sh local` -> `bin/cli.sh process start local` `bin/alluxio-stop.sh masters` -> `bin/cli.sh process stop masters` `bin/alluxio-stop.sh job_masters` -> `bin/cli.sh process stop job_masters` `bin/alluxio-stop.sh workers` -> `bin/cli.sh process stop workers` `bin/alluxio-stop.sh job_workers` -> `bin/cli.sh process stop job_workers` `bin/alluxio-stop.sh proxies` -> `bin/cli.sh process stop proxies` `bin/alluxio-stop.sh all` -> `bin/cli.sh process stop all` `bin/alluxio-stop.sh local` -> `bin/cli.sh process stop local` For command `process start/stop` on multiple nodes, using crypto's `ssh` package to create an SSH session, connect to masters, workers or all nodes, then send according subcommand on single nodes to these nodes. pr-link: #17887 change-id: cid-0a660de35945738c065dae2f008a61d48f6b3be9

Add `process` commands with multiple nodes to golang CLI as part of Alluxio#17522 `bin/alluxio-start.sh masters` -> `bin/cli.sh process start masters` `bin/alluxio-start.sh job_masters` -> `bin/cli.sh process start job_masters` `bin/alluxio-start.sh workers` -> `bin/cli.sh process start workers` `bin/alluxio-start.sh job_workers` -> `bin/cli.sh process start job_workers` `bin/alluxio-start.sh proxies` -> `bin/cli.sh process start proxies` `bin/alluxio-start.sh all` -> `bin/cli.sh process start all` `bin/alluxio-start.sh local` -> `bin/cli.sh process start local` `bin/alluxio-stop.sh masters` -> `bin/cli.sh process stop masters` `bin/alluxio-stop.sh job_masters` -> `bin/cli.sh process stop job_masters` `bin/alluxio-stop.sh workers` -> `bin/cli.sh process stop workers` `bin/alluxio-stop.sh job_workers` -> `bin/cli.sh process stop job_workers` `bin/alluxio-stop.sh proxies` -> `bin/cli.sh process stop proxies` `bin/alluxio-stop.sh all` -> `bin/cli.sh process stop all` `bin/alluxio-stop.sh local` -> `bin/cli.sh process stop local` For command `process start/stop` on multiple nodes, using crypto's `ssh` package to create an SSH session, connect to masters, workers or all nodes, then send according subcommand on single nodes to these nodes. pr-link: Alluxio#17887 change-id: cid-0a660de35945738c065dae2f008a61d48f6b3be9

- Add cli/ folder containing golang code - Add script to compile code to executable in build/cli/build-cli.sh - Add entrypoint script for development in bin/cli.sh - Add build profile to build the CLI as part of tha maven build via `-PgoCli` - Add `conf` service, consisting of - `bin/cli.sh conf get` as an example command, equivalent to `bin/alluxio getConf` - `bin/cli.sh conf log --name fully.qualified.class.path`, equivalent to `bin/alluxio fsadmin logLevel --logName fully.qualified.class.path` See #17522 for more background info pr-link: #17532 dev docs for defining conventions for the new alluxio cli in golang pr-link: #17530 - Add commands to start/stop individual processes, ex. `bin/cli.sh process start master` - Define process interface and registry - process specific java opts are defined with the process and are dynamically added to env - Add journal format command as part of starting master process - Mount options for worker are not ported because they will be deprecated in the near future pr-link: #17561 Adds `info` subcommand, which contains: - `cache`: worker capacity information, calls the existing `bin/alluxio fsadmin report capacity` command - `collect`: collects cluster information into a single tarball, calls the existing `bin/alluxio collectInfo` command - `master`: master quorum information, calls the existing `bin/alluxio fs masterInfo` command - `report`: cluster information, calls the existing `bin/alluxio fsadmin report` command, excluding the `capacity` category pr-link: #17566 Add journal commands to CLI as part of #17522 pr-link: #17569 Add quorum/HA related commands as part of #17522 pr-link: #17570 Add fs commands to golang CLI as part of #17522 unlike other CLI commands, utilize arguments to maintain the existing structure of filesystem commands (ex. cp, du, ls, mv, rm, etc) pr-link: #17580 Add generate commands to golang CLI as part of #17522 bin/alluxio docGen -> bin/cli generate docs bin/alluxio bootstrapConf -> bin/cli generate template to read ALLUXIO_CONF_DIR, also conducted a minor change on env variables. pr-link: #17790 Add exec commands to golang CLI as part of #17522 `bin/alluxio runClass` -> `bin/cli exec class` `bin/alluxio runTests` -> `bin/cli exec testRun` `bin/alluxio runUfsTests` -> `bin/cli exec testUfs` `bin/alluxio runUfsIOTest` -> `bin/cli exec testUfsIO` `bin/alluxio runHdfsMountTests` -> `bin/cli exec testHdfsMount` `bin/alluxio runHmsTests` -> `bin/cli exec testHms` `bin/alluxio runJournalCrashTest` -> `bin/cli exec testJournalCrash` For command `exec test`, using different `--name` flags can lead to huge difference in other flags and options. To manage flags and options, currently using different commands for different test types. pr-link: #17797 Prepare golang cli for tarball build and replace bin/alluxio allow java 8 or 11 add cli binaries to other tarball profiles Add `process` commands with multiple nodes to golang CLI as part of #17522 `bin/alluxio-start.sh masters` -> `bin/cli.sh process start masters` `bin/alluxio-start.sh job_masters` -> `bin/cli.sh process start job_masters` `bin/alluxio-start.sh workers` -> `bin/cli.sh process start workers` `bin/alluxio-start.sh job_workers` -> `bin/cli.sh process start job_workers` `bin/alluxio-start.sh proxies` -> `bin/cli.sh process start proxies` `bin/alluxio-start.sh all` -> `bin/cli.sh process start all` `bin/alluxio-start.sh local` -> `bin/cli.sh process start local` `bin/alluxio-stop.sh masters` -> `bin/cli.sh process stop masters` `bin/alluxio-stop.sh job_masters` -> `bin/cli.sh process stop job_masters` `bin/alluxio-stop.sh workers` -> `bin/cli.sh process stop workers` `bin/alluxio-stop.sh job_workers` -> `bin/cli.sh process stop job_workers` `bin/alluxio-stop.sh proxies` -> `bin/cli.sh process stop proxies` `bin/alluxio-stop.sh all` -> `bin/cli.sh process stop all` `bin/alluxio-stop.sh local` -> `bin/cli.sh process stop local` For command `process start/stop` on multiple nodes, using crypto's `ssh` package to create an SSH session, connect to masters, workers or all nodes, then send according subcommand on single nodes to these nodes. pr-link: #17887 Add job commands to golang CLI as part of #17522 `bin/alluxio-bash job cancel id` -> `bin/alluxio job cancel --id` `bin/alluxio-bash job leader` -> `bin/alluxio job leader` `bin/alluxio-bash job ls` -> `bin/alluxio job list` `bin/alluxio-bash job getCmdStatus jobControlId` -> `bin/alluxio job cmdStatus --id` `bin/alluxio-bash job stat [-v] id` -> `bin/alluxio job jobStatus [-v] --id` `bin/alluxio-bash fs distributedCp src dst` -> `bin/alluxio job submit --type cp --src --dst` `bin/alluxio-bash fs distributedMv src dst` -> `bin/alluxio job submit --type mv --src --dst` `bin/alluxio-bash fs load path --submit` -> `bin/alluxio job load --path` pr-link: #17931 Clean up multiprocess code - refactor commands launching the same process on multiple hosts into a single struct - simplify the logic to ssh into different hosts with more straightforward handling of error channel - move names.go under its own package to avoid import cycles pr-link: #17947 Add an `info version` subcommand to golang CLI as part of #17522 `bin/alluxio-bash version` -> `bin/alluxio info version` pr-link: #17943 update calls to old bash scripts to refer to alluxio-bash Add `cache` commands to golang CLI as part of #17522 `bin/alluxio-bash formatWorker` -> `bin/cli cache format` `bin/alluxio-bash fs freeWorker` -> `bin/cli cache free --worker` `bin/alluxio-bash fs free` -> `bin/cli cache free [-f] --path` `bin/alluxio-bash fs persist` -> `bin/cli cache persist --path` `bin/alluxio-bash fs setTtl` -> `bin/cli cache ttl --set --duration --action` `bin/alluxio-bash fs unsetTtl` -> `bin/cli cache ttl --unset` pr-link: #17937 backcompat support for start/stop scripts skip mount args in start script backcompat remove unsupported cmds

twalluxio added 18 commits August 4, 2023 09:29

resolved untrimmed path on finding which java

a77f8dd

guidelines for workers.go

8a60780

add entry for command workers, do nothing

4c0b8b4

can ssh to workers and execute start worker commands

d2c0a3d

fix typo, add comments and to-dos

ee9306d

able to execute commands on workers

fe2ccea

able to complete worker start on workers

c014ebf

re-stucture and polish workers.go

3fd38cd

polished code, support process start flags

bfa1a87

create proxies.go, do nothing

37dc475

create masters.go, do nothing

aac60e9

add start/stop commands for process start/stop masters

b862cdc

add start/stop commands for process start/stop proxies

41e45db

register masters and proxies commands

10bb938

move common functions to common.go

cdbdeda

restructured runCommands(), allow create multiple sessions

3f0e60e

create and register job_masters.go, job_workers.go

8f3bbc5

create and register all.go, local.go

24b44cf

twalluxio added 2 commits August 4, 2023 14:54

change default path and add flags for stop commands

814cfe7

rewrite local and all commands

4daa26d

twalluxio force-pushed the golangCli-rsync3 branch from 46e0987 to 4daa26d Compare August 4, 2023 06:55

twalluxio changed the title ~~[In progress] Add process subcommands for multiple nodes~~ Add process subcommands for multiple nodes Aug 4, 2023

twalluxio marked this pull request as ready for review August 4, 2023 07:34

improve code and log readability

946de5d

Kai-Zhang reviewed Aug 4, 2023

View reviewed changes

Xenorith requested changes Aug 4, 2023

View reviewed changes

Xenorith reviewed Aug 4, 2023

View reviewed changes

Cleanup procedure

872ba01

twalluxio added 3 commits August 7, 2023 11:55

combine getMasters() and getWorkers() to getNodes(bool), move cliPath…

58a59f6

… to common.go

rewrite start/stop methods for less redundancy

281dbe7

configure username and ssh port

8c28f25

Xenorith requested changes Aug 7, 2023

View reviewed changes

use go channel & goroutine to trigger ssh commands

0b7522c

Kai-Zhang reviewed Aug 7, 2023

View reviewed changes

twalluxio added 2 commits August 7, 2023 17:21

restructured common.go

e18573c

add sync lock for reading hostnames

2521dd8

Xenorith requested changes Aug 7, 2023

View reviewed changes

small feature fix; avoid use magic strings

ac5bf44

twalluxio requested a review from Xenorith August 8, 2023 02:28

Xenorith reviewed Aug 8, 2023

View reviewed changes

twalluxio and others added 2 commits August 8, 2023 10:33

join with space " " instead of empty string ""

0fd9cb1

Merge branch 'golangCli' into golangCli-rsync3

390c717

Xenorith approved these changes Aug 8, 2023

View reviewed changes

Kai-Zhang added the type-feature This issue is a feature request label Aug 8, 2023

alluxio-bot merged commit 8a8b338 into Alluxio:golangCli Aug 8, 2023
11 checks passed

twalluxio deleted the golangCli-rsync3 branch August 14, 2023 04:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add process subcommands for multiple nodes #17887

Add process subcommands for multiple nodes #17887

twalluxio commented Aug 4, 2023

alluxio-bot commented Aug 4, 2023

alluxio-bot commented Aug 4, 2023

Xenorith Aug 4, 2023

twalluxio Aug 7, 2023

Xenorith left a comment

Xenorith Aug 7, 2023

Kai-Zhang Aug 7, 2023

Kai-Zhang Aug 7, 2023

Xenorith left a comment

Xenorith left a comment

Xenorith commented Aug 8, 2023

alluxio-bot commented Aug 8, 2023

Kai-Zhang commented Aug 8, 2023

Add process subcommands for multiple nodes #17887

Add process subcommands for multiple nodes #17887

Conversation

twalluxio commented Aug 4, 2023

alluxio-bot commented Aug 4, 2023

alluxio-bot commented Aug 4, 2023

Xenorith Aug 4, 2023

Choose a reason for hiding this comment

twalluxio Aug 7, 2023

Choose a reason for hiding this comment

Xenorith left a comment

Choose a reason for hiding this comment

Xenorith Aug 7, 2023

Choose a reason for hiding this comment

Kai-Zhang Aug 7, 2023

Choose a reason for hiding this comment

Kai-Zhang Aug 7, 2023

Choose a reason for hiding this comment

Xenorith left a comment

Choose a reason for hiding this comment

Xenorith left a comment

Choose a reason for hiding this comment

Xenorith commented Aug 8, 2023

alluxio-bot commented Aug 8, 2023

Kai-Zhang commented Aug 8, 2023