Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 15, 2025

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Addresses review feedback from #24075 to decouple throttle target and throttle type in the ctl interface.

Previously, the ctl interface inferred throttle type from throttle target (e.g., sink target → sink type). This prevented scenarios like applying backfill throttling to sinks.

Changes:

  • Added required --throttle-type option accepting dml, backfill, source, sink (case-insensitive)
  • Added sink subcommand to throttle commands
  • Simplified implementation by removing backward compatibility logic

Usage:

# Users must always explicitly specify the throttle type
risectl throttle sink 123 1000 --throttle-type backfill
risectl throttle mv 123 1000 --throttle-type backfill
risectl throttle source 123 1000 --throttle-type source

This allows independent specification of throttle target and type, enabling scenarios like sink backfill rate limiting.

Checklist

  • I have written necessary rustdoc comments.
  • I have added necessary unit tests and integration tests.
  • I have added test labels as necessary.
  • I have added fuzzing tests or opened an issue to track them.
  • My PR contains breaking changes.
  • My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
  • I have checked the Release Timeline and Currently Supported Versions to determine which release branches I need to cherry-pick this PR into.

Documentation

  • My PR needs documentation updates.
Release note

The risectl throttle command now requires the --throttle-type flag to explicitly specify throttle type independently of the target. This enables scenarios like applying backfill throttling to sinks.

Breaking change: The --throttle-type option is now required for all throttle commands.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Update ctl interface to add new CLI option for sink backfill feat(ctl): add explicit throttle type option for independent target/type control Dec 15, 2025
Copilot AI requested a review from kwannoel December 15, 2025 05:58
Comment on lines 27 to 64
// 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
}
}
};

Copy link
Contributor

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

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

Copilot AI changed the title feat(ctl): add explicit throttle type option for independent target/type control feat(ctl): add required throttle type option for independent target/type control Dec 15, 2025
Copilot AI requested a review from kwannoel December 15, 2025 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/feature Type: New feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants