-
Notifications
You must be signed in to change notification settings - Fork 555
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
feat(cmd_all): support single_node
mode
#14951
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
license-eye has totally checked 4785 files.
Valid | Invalid | Ignored | Fixed |
---|---|---|---|
2081 | 1 | 2703 | 0 |
Click to see the invalid file list
- src/cmd_all/src/single.rs
027206e
to
d84d076
Compare
6560863
to
a57cf07
Compare
|
||
pub fn make_single_node_sql_endpoint(store_directory: &String) -> String { | ||
format!( | ||
"sqlite://{}/meta_store/single_node.db?mode=rwc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"sqlite://{}/meta_store/single_node.db?mode=rwc", | |
"sqlite://{}/meta_store.db?mode=rwc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm why this change? IMO this structure looks more consistent:
noelkwan@Noels-MacBook-Pro risingwave % tree ~/.risingwave/
/Users/noelkwan/.risingwave/
├── meta_store
│ └── single_node.db
└── state_store
└── hummock_001
├── checkpoint
│ └── 0
└── cluster_id
└── 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Than this:
noelkwan@Noels-MacBook-Pro risingwave % tree ~/.risingwave/
/Users/noelkwan/.risingwave/
├── single_node.db
└── state_store
└── hummock_001
├── checkpoint
│ └── 0
└── cluster_id
└── 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Eric's idea is:
/Users/noelkwan/.risingwave/
├── meta_store.db
└── state_store
└── hummock_001
├── checkpoint
│ └── 0
└── cluster_id
└── 0
Will take a look next weekday. 🥺 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now we have 3 single-process deployment modes:
playground
: all config managedsingle-node
: some config managedstandalone
: almost no config managed
Intuitively, they're different from each other and exist for some different demand. IMO, however, they could be understood as a single mode with different sets of command-line options being set to the default value.
For example, imagine that we have a single
mode like this:
risingwave single
[--preset <profile>]
# fields that will be mapped, use default value if absent
[--meta-address ".."]
[--config-file ".."]
# Extra options to override those from the preset
[--meta-extra-opts ".."]
[--compute-extra-opts ".."]
[--frontend-extra-opts ".."]
[--compactor-extra-opts ".."]
Then...
playground
issingle --preset in-memory
single-node
issingle --preset local-disk
standalone
issingle
withoutpreset
and specify the address and extra options manually.
In this approach there's not a clear boundary of 3 modes. If users want to tweak a few options for playground
or single-node
, they don't have to fall back to standalone
. Just simply provide some extra-opts
.
BTW, the map_opts
seems to actually achieve the idea I proposed for improving the UX for standalone
mode in #12142 (comment)
For example, if we have specified the listen address of the meta service, then we don't need to specify the meta address again for compute and frontend services. That's why I propose this issue: perhaps we need a (cluster-level ?) configuration file for this as a better replacement for raw command-line arguments.
It's available only for single-node
under current design, but I still believe it's a good to have for standalone
. With the unified single --preset
approach, I guess all single deployment modes can benefit from this.
} | ||
|
||
impl SingleNodeOpts { | ||
fn default_frontend_opts() -> FrontendOpts { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO these methods could be painful to maintain. I'd suggest specifying the mapped fields only and leave other fields default with ..Default::default()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm not sure how to expose clap's defaults. e.g. the default values specified by default_value
.
I think I tried ..Default::default()
for FrontendOpts
previously, but it gives an error that the trait is not implemented for FrontendOpts
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. Thought it's serde
.
#[command( | ||
version, | ||
about = "The Standalone mode allows users to start multiple services in one process, it exposes node-level options for each service", | ||
hide = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are now hiding the entrance of standalone
, it may be more appropriate to assign the name to single_node
instead, as discussed. 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename single_node
-> standalone
? Not quite sure what you mean.
We hide it from the end user, but cloud will still invoke it with ./risingwave standalone
.
Some considerations:
Otherwise, seems like this can unify all 3 execution modes and reduce maintenance overhead. |
Some extra work needs to be done:
Proc macro output:
Thanks @BugenZhao for mentioning: |
Discussed offline with bugen, we can merge this PR, and update the internals later to unify Track: #14997 |
Co-authored-by: Noel Kwan <47273164+kwannoel@users.noreply.github.com> Co-authored-by: Noel Kwan <noelkwan1998@gmail.com>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
single_node
mode is a new mode for users who wish to run risingwave in a single process.Previously users would rely on etcd and minio to run such an instance, now we use file system and embedded sqlite to power the state store and meta stores.
For the configurations of
single_node
, we don't need to exposenode_level
options to the end-user. As such we create a new options struct with the essential options.For the internals, we just:
standalone_mode
.standalone_mode
to spawn all these nodes in a single process.Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
This is considered a "pre-release".
In this PR, we release a new mode:
single_node
mode../risingwave
will invoke this new mode by default.Setup
You can run it simply with the following steps:
git clone git@github.com:risingwavelabs/risingwave.git git checkout kwannoel/standalone-easy-defaults cd risingwave ./risedev build-risingwave ./target/debug/risingwave
Trying it
In a separate terminal connect to risingwave:
Cleaning the cluster
The default data directory is in
~/.risingwave
. You canrm
that directory to purge associated data.Configurations
Hiding other features