Skip to content

feat(ctl): add type-safe throttle control with named CLI arguments #24111

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.

Sign up for GitHub

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

Merged
kwannoel merged 6 commits into from
Dec 17, 2025
Merged

feat(ctl): add type-safe throttle control with named CLI arguments #24111

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3ed52c4
Initial plan
Copilot Dec 15, 2025
8b2cb88
Add throttle-type CLI option and sink subcommand for throttle control
Copilot Dec 15, 2025
a5f18a5
Apply code review feedback: use eq_ignore_ascii_case and simplify pat…
Copilot Dec 15, 2025
14ee937
Simplify: make --throttle-type required, remove backward compatibility
Copilot Dec 15, 2025
eaf1236
Make CLI args consistent: convert id and rate to named flags
Copilot Dec 17, 2025
d3d1dd6
Use enum for throttle-type to improve type safety and CLI UX
Copilot Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 36 additions & 8 deletions src/ctl/src/cmd_impl/throttle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,56 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use anyhow::bail;
use risingwave_pb::common::PbThrottleType;
use risingwave_pb::meta::PbThrottleTarget;

use crate::ThrottleCommandArgs;
use crate::common::CtlContext;

/// Parse throttle type from string
fn parse_throttle_type(s: &str) -> anyhow::Result<PbThrottleType> {
if s.eq_ignore_ascii_case("dml") {
Ok(PbThrottleType::Dml)
} else if s.eq_ignore_ascii_case("backfill") {
Ok(PbThrottleType::Backfill)
} else if s.eq_ignore_ascii_case("source") {
Ok(PbThrottleType::Source)
} else if s.eq_ignore_ascii_case("sink") {
Ok(PbThrottleType::Sink)
} else {
bail!(
"Invalid throttle type: {}. Valid options are: dml, backfill, source, sink",
s
)
}
}

pub async fn apply_throttle(
context: &CtlContext,
kind: PbThrottleTarget,
params: ThrottleCommandArgs,
) -> anyhow::Result<()> {
let meta_client = context.meta_client().await?;
// Infer throttle type from target for backward compatibility.
let throttle_type = match kind {
PbThrottleTarget::Source => PbThrottleType::Source,
PbThrottleTarget::Mv => PbThrottleType::Backfill,
PbThrottleTarget::Sink => PbThrottleType::Sink,
PbThrottleTarget::Table | PbThrottleTarget::Fragment | PbThrottleTarget::Unspecified => {
// Default to Backfill for unspecified/unsupported combinations; user should use SQL for table throttling
PbThrottleType::Backfill

// Use provided throttle type if specified, otherwise infer from target for backward compatibility
let throttle_type = if let Some(type_str) = &params.throttle_type {
parse_throttle_type(type_str)?
} else {
// Infer throttle type from target for backward compatibility
match kind {
PbThrottleTarget::Source => PbThrottleType::Source,
PbThrottleTarget::Mv => PbThrottleType::Backfill,
PbThrottleTarget::Sink => PbThrottleType::Sink,
PbThrottleTarget::Table
| PbThrottleTarget::Fragment
| PbThrottleTarget::Unspecified => {
// Default to Backfill for unspecified/unsupported combinations; user should use SQL for table throttling
PbThrottleType::Backfill
}
}
};

Comment on lines 27 to 33
Copy link
Contributor

@kwannoel kwannoel Dec 15, 2025

Choose a reason for hiding this comment

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

@copilot no need to maintain backwards compatibliilty, pkease simplify.

Copy link
Contributor Author

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

Simplified the code in commit 14ee937. Made --throttle-type required and removed all backward compatibility logic.

The apply_throttle function is now just 3 lines, and users must always specify the throttle type explicitly:

risectl throttle sink <id> <rate> --throttle-type backfill

Copy link
Contributor

@kwannoel kwannoel Dec 17, 2025

Choose a reason for hiding this comment

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

@copilot
Please make the cli args consistent as well, provide:

  • id cli arg.
  • rate cli arg.

Copy link
Contributor Author

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

Made CLI args consistent in commit eaf1236. Both id and rate are now named flags:

risectl throttle sink --id <ID> --rate <RATE> --throttle-type <TYPE>

Help output:

Usage: risectl throttle sink [OPTIONS] --id <ID> --throttle-type <TYPE>

Options:
      --id <ID>               The ID of the object to throttle
      --rate <RATE>           The rate limit to apply
      --throttle-type <TYPE>  The type of throttle to apply. Options: dml, backfill, source, sink

meta_client
.apply_throttle(kind, throttle_type, params.id, params.rate)
.await?;
Expand Down
7 changes: 7 additions & 0 deletions src/ctl/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,12 +506,16 @@ enum TestCommands {
enum ThrottleCommands {
Source(ThrottleCommandArgs),
Mv(ThrottleCommandArgs),
Sink(ThrottleCommandArgs),
}

#[derive(Clone, Debug, Args)]
pub struct ThrottleCommandArgs {
id: u32,
rate: Option<u32>,
/// The type of throttle to apply. Options: dml, backfill, source, sink
#[clap(long, value_name = "TYPE")]
throttle_type: Option<String>,
}

#[derive(Subcommand, Clone, Debug)]
Expand Down Expand Up @@ -921,6 +925,9 @@ async fn start_impl(opts: CliOpts, context: &CtlContext) -> Result<()> {
Commands::Throttle(ThrottleCommands::Mv(args)) => {
apply_throttle(context, risingwave_pb::meta::PbThrottleTarget::Mv, args).await?;
}
Commands::Throttle(ThrottleCommands::Sink(args)) => {
apply_throttle(context, risingwave_pb::meta::PbThrottleTarget::Sink, args).await?;
}
Commands::Meta(MetaCommands::SetCdcTableBackfillParallelism {
table_id,
parallelism,
Expand Down
Loading