Skip to content
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

fix: correctly implement default subcommand for single-node #16080

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

BugenZhao
Copy link
Member

Signed-off-by: Bugen Zhao i@bugenzhao.comI hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

In #15847 and #15897 we made single-node the default in CLI if there's no other explicit subcommand specified. However, the approach that matches on the error kind leads to some unexpected side effects like #16065.

This PR utilizes a more recommended approach (ref) to achieve the same behavior, which resolves #16065 and also improves the help messages like the following:

All-in-one executable for components of RisingWave

Usage: risingwave [OPTIONS]
       risingwave <COMPONENT>

Components:
  compute      The worker node that executes query plans and handles data ingestion and output [aliases: compute-node, compute_node]
  meta         The central metadata management service [aliases: meta-node, meta_node]
  frontend     The stateless proxy that parses SQL queries and performs planning and optimizations of query jobs [aliases: frontend-node, frontend_node]
  compactor    The stateless worker node that compacts data for the storage engine [aliases: compactor-node, compactor_node]
  ctl          The DevOps tool that provides internal access to the RisingWave cluster [aliases: risectl]
  single_node  [default] The Single Node mode. Start all services in one process, with process-level options. This will be executed if no subcommand is specified [aliases: single-node, single]
  help         Print this message or the help of the given subcommand(s)

Options:
      --prometheus-listener-addr <PROMETHEUS_LISTENER_ADDR>
          The address prometheus polls metrics from [env: RW_SINGLE_NODE_PROMETHEUS_LISTENER_ADDR=]
      --config-path <CONFIG_PATH>
          The path to the cluster configuration file [env: RW_SINGLE_NODE_CONFIG_PATH=]
      --store-directory <STORE_DIRECTORY>
          The store directory used by meta store and object store [env: RW_SINGLE_NODE_STORE_DIRECTORY=]
      --in-memory
          Start RisingWave in-memory [env: RW_SINGLE_NODE_IN_MEMORY=]
      --total-memory-bytes <TOTAL_MEMORY_BYTES>
          Total available memory for the compute node in bytes. Used by both computing and storage
      --parallelism <PARALLELISM>
          The parallelism that the compute node will register to the scheduler of the meta service
      --listen-addr <LISTEN_ADDR>
          The address that this service listens to. Usually the localhost + desired port
      --prometheus-endpoint <PROMETHEUS_ENDPOINT>
          The HTTP REST-API address of the Prometheus instance associated to this cluster. This address is used to serve `PromQL` queries to Prometheus. It is also used by Grafana Dashboard Service to fetch metrics and visualize them
      --prometheus-selector <PROMETHEUS_SELECTOR>
          The additional selector used when querying Prometheus
      --compaction-worker-threads-number <COMPACTION_WORKER_THREADS_NUMBER>
          
  -h, --help
          Print help (see more with '--help')
  -V, --version
          Print version

Also add some tests to enforce the parsing behavior.

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

  • My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@github-actions github-actions bot added the type/fix Bug fix label Apr 2, 2024
@BugenZhao BugenZhao requested review from lmatz, fuyufjh and kwannoel April 2, 2024 07:00
Copy link
Contributor

@kwannoel kwannoel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@BugenZhao BugenZhao added this pull request to the merge queue Apr 2, 2024
Merged via the queue into main with commit c2b0f58 Apr 2, 2024
29 of 40 checks passed
@BugenZhao BugenZhao deleted the bz/better-default-subcommand branch April 2, 2024 08:28
github-actions bot pushed a commit that referenced this pull request Apr 2, 2024
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
github-merge-queue bot pushed a commit that referenced this pull request Apr 2, 2024
#16086)

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Co-authored-by: Bugen Zhao <i@bugenzhao.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

./target/debug/risingwave ctl meta unregister-worker 4 returns wrong error message
3 participants