Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 4, 2025

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

What's changed and what's your intention?

The previous ALTER FRAGMENT SET RATE_LIMIT had ambiguous semantics when a fragment has both sink and backfill rate limits. This PR decouples the single RATE_LIMIT into four specific types.

New syntax:

ALTER FRAGMENT <id> SET BACKFILL_RATE_LIMIT TO <value>;
ALTER FRAGMENT <id> SET SOURCE_RATE_LIMIT TO <value>;
ALTER FRAGMENT <id> SET DML_RATE_LIMIT TO <value>;
ALTER FRAGMENT <id> SET SINK_RATE_LIMIT TO <value>;

Changes:

  • AlterFragmentOperation enum: replaced AlterBackfillRateLimit with four specific variants (SetBackfillRateLimit, SetSourceRateLimit, SetDmlRateLimit, SetSinkRateLimit)
  • Parser: reuses existing rate limit parsing methods instead of a separate parse_alter_fragment_rate_limit
  • Handler: all four types dispatch to the same PbThrottleTarget::Fragment backend

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

Breaking change: ALTER FRAGMENT <id> SET RATE_LIMIT syntax has been replaced with specific rate limit types:

  • ALTER FRAGMENT <id> SET BACKFILL_RATE_LIMIT TO <value>
  • ALTER FRAGMENT <id> SET SOURCE_RATE_LIMIT TO <value>
  • ALTER FRAGMENT <id> SET DML_RATE_LIMIT TO <value>
  • ALTER FRAGMENT <id> SET SINK_RATE_LIMIT TO <value>
Original prompt

This section details on the original issue you should resolve

<issue_title>Refactor: Change ALTER FRAGMENT SET RATE_LIMIT to ALTER FRAGMENT SET (BACKFILL_RATE_LIMIT | SOURCE_RATE_LIMIT | DML_RATE_LIMIT | SINK_RATE_LIMIT)</issue_title>
<issue_description>As per title. Currently we just lump it into a single alter fragment set rate limit, however if a fragment has both sink rate limit and backfill rate limit, the semantics of alter becomes ambiguous. It's better to decouple these.</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Change AlterFragmentOperation enum to support BACKFILL_RATE_LIMIT, SOURCE_RATE_LIMIT, DML_RATE_LIMIT, and SINK_RATE_LIMIT
- Update parser to parse new rate limit types
- Update handler to handle new rate limit operations
- Update e2e test to use new BACKFILL_RATE_LIMIT syntax
- Add unit tests for new rate limit parsing

Co-authored-by: kwannoel <47273164+kwannoel@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor alter fragment set rate limit semantics Refactor: Change ALTER FRAGMENT SET RATE_LIMIT to support specific rate limit types Dec 4, 2025
Copilot AI requested a review from kwannoel December 4, 2025 10:23
Copilot finished work on behalf of kwannoel December 4, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: Change ALTER FRAGMENT SET RATE_LIMIT to ALTER FRAGMENT SET (BACKFILL_RATE_LIMIT | SOURCE_RATE_LIMIT | DML_RATE_LIMIT | SINK_RATE_LIMIT)

2 participants