CELEBORN-1689: CLI Support for Ratis APIs #2882
Conversation
|
@akpatnam25, why not directly use ratis shell of ratis? The REST API of ratis is same as the ratis shell. |
To use ratis shell, you need to be on the host itself, and also have access to the distribution to run it. I think in most production deployments, it is not convenient (and sometimes not even possible) to access the host and run it within the host. I think from operational point of view, it is better to use the REST API via the CLI since it can be accessed from anywhere outside host. Let me know what you think :) @SteNicholas |
|
@akpatnam25, we would use ratis-shell in master node for our production pratice. Meanwhile, CLI does not depend on the REST API, otherwise why not directly invoke the REST API? |
@SteNicholas CLI does depend on REST Api. It calls the REST api. Not all users would have access to their production host, and would only have access to REST api (CLI). |
|
cc @FMX |
|
cc @turboFei |
@akpatnam25, how does the users choose ratis-shell or this CLI? Meanwhile, not all users would have access to REST API, because CLI needs users to provide the host and port via |
|
@SteNicholas I think for the ratis API it makes sense to have cli support for them and we can deprecate the ratis-shell if that is ok with the community later on. We don’t need to support for all rest api’s but I think this one is especially important given the only other way to do it is to log onto a production host. We do not need cli support for every single rest api, just the main ones that help with operational management. Let me know :) thanks. |
| @@ -1,40 +1,36 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <!-- | |||
| @@ -120,4 +120,46 @@ final class MasterOptions { | |||
| names = Array("--delete-apps"), | |||
| description = Array("Delete resource of an application.")) | |||
| private[master] var deleteApps: Boolean = _ | |||
|
|
|||
There was a problem hiding this comment.
There are two more commands to implement.
- show raft group info
- print local raft meta conf
| captureOutputAndValidateResponse( | ||
| args, | ||
| "", | ||
| "org.apache.celeborn.service.deploy.master.clustermeta.SingleMasterMetaManager" + |
There was a problem hiding this comment.
This test is invlid. You can setup a minu cluster to test ratis cluster.
There was a problem hiding this comment.
You can refer to MiniClusterFeature.
| private[master] var setRatisPeersPriorities: Boolean = _ | ||
|
|
||
| @Option( | ||
| names = Array("--create-snapshot"), |
There was a problem hiding this comment.
how about using similar command names?
--election-transfer
--election-step-down
--election-pause
--election-resume
--peer-add
--peer-remove
--peer-set-priority
--snapshot-create
--local-raft-conf
|
spoke more with @SteNicholas - given that ratis shell is the source of truth, we can just keep it like that. We would not need CLI support for these. Closing out this PR. thanks for the feedback to everyone |
What changes were proposed in this pull request?
Add CLI support for Ratis REST APIs that were recently added in https://issues.apache.org/jira/browse/CELEBORN-1628
Leaving out local raft meta conf APIs for no since those are not as straightforward. Will update those in another PR.
Why are the changes needed?
better ops since with REST API and CLI there is no need to log onto production hosts to manage ratis operations.
Does this PR introduce any user-facing change?
No
How was this patch tested?
added unit tests