feat: support backfill_rate_limit for source backfill #19445

xxchan · 2024-11-19T06:56:38Z

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

What's changed and what's your intention?

fix #19297

This is a missing feature. We support source_rate_limit for shared source executor, but cannot rate limit source backfill before this PR.

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
I have added test labels as necessary. See details.
I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
All checks passed in ./risedev check (or alias, ./risedev c)
My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

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.

xxchan · 2024-11-19T06:56:54Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

xxchan · 2024-11-19T07:14:23Z

src/meta/src/controller/streaming_job.rs

@@ -1412,7 +1411,7 @@ impl CatalogController {
        fragments.retain_mut(|(_, fragment_type_mask, stream_node)| {
            let mut found = false;
            if (*fragment_type_mask & PbFragmentTypeFlag::StreamScan as i32 != 0)
-                || (*fragment_type_mask & PbFragmentTypeFlag::Source as i32 != 0)


The previous code looks wrong: Alter backfill rate limit will affect MV on non-shared source

tabVersion · 2024-11-19T09:26:19Z

Please also leave some doc on how to use the source_rate_limit and backfill_rate_limit from the user's perspective. It is really confusing.

xxchan · 2024-11-19T09:26:42Z

risingwavelabs/risingwave-docs-new#62

kwannoel

LGTM, thanks!

tabVersion

rest LGTM

tabVersion · 2024-11-19T09:27:24Z

proto/stream_plan.proto

+  // Backfill rate limit
  optional uint32 rate_limit = 7;


shall we directly change the field name for distinguish?

It will be breaking change

why? It keeps the same tag number. https://stackoverflow.com/questions/45431685/protocol-buffer-does-changing-field-name-break-the-message

IIRC sql meta backend stores with JSON encoding

tabVersion · 2024-11-19T09:35:43Z

src/meta/src/controller/streaming_job.rs

@@ -1412,7 +1411,7 @@ impl CatalogController {
        fragments.retain_mut(|(_, fragment_type_mask, stream_node)| {
            let mut found = false;
            if (*fragment_type_mask & PbFragmentTypeFlag::StreamScan as i32 != 0)
-                || (*fragment_type_mask & PbFragmentTypeFlag::Source as i32 != 0)


xxchan · 2024-11-20T07:57:58Z

Merge activity

Nov 20, 3:57 PM GMT+8: A user started a stack merge that includes this pull request via Graphite.
Nov 20, 3:58 PM GMT+8: Graphite couldn't merge this pull request because a downstack PR fix: fix potential data loss for shared source #19443 failed to merge.
Nov 20, 6:53 PM GMT+8: Graphite rebased this pull request after merging its parent, because this pull request is set to merge when ready.
Nov 20, 7:34 PM GMT+8: Graphite couldn't merge this PR because it failed for an unknown reason (This repository has GitHub's merge queue enabled, which is currently incompatible with Graphite).
Nov 20, 7:35 PM GMT+8: Graphite couldn't merge this PR because it failed for an unknown reason (This repository has GitHub's merge queue enabled, which is currently incompatible with Graphite).

…19466) (#19482) Signed-off-by: xxchan <[email protected]> Co-authored-by: Noel Kwan <[email protected]>

github-actions bot added type/feature ci/run-e2e-single-node-tests ci/run-e2e-test-other-backends labels Nov 19, 2024

xxchan marked this pull request as ready for review November 19, 2024 06:56

xxchan mentioned this pull request Nov 19, 2024

fix: fix potential data loss for shared source #19443

Merged

9 tasks

xxchan force-pushed the 11-18-fix_fix_potential_data_loss_for_shared_source branch from a29b177 to 13725fc Compare November 19, 2024 07:12

xxchan force-pushed the 11-16-feat_support_backfill_rate_limit_for_source_backfill branch from 56d29e1 to 60bdf58 Compare November 19, 2024 07:12

xxchan commented Nov 19, 2024

View reviewed changes

xxchan force-pushed the 11-18-fix_fix_potential_data_loss_for_shared_source branch from 13725fc to bdf47a5 Compare November 19, 2024 07:16

xxchan force-pushed the 11-16-feat_support_backfill_rate_limit_for_source_backfill branch from 60bdf58 to 35c1612 Compare November 19, 2024 07:16

xxchan force-pushed the 11-18-fix_fix_potential_data_loss_for_shared_source branch from bdf47a5 to 1789be9 Compare November 19, 2024 07:43

xxchan force-pushed the 11-16-feat_support_backfill_rate_limit_for_source_backfill branch from 35c1612 to c4b4976 Compare November 19, 2024 07:43

xxchan force-pushed the 11-18-fix_fix_potential_data_loss_for_shared_source branch from 1789be9 to 39887aa Compare November 19, 2024 07:43

xxchan force-pushed the 11-16-feat_support_backfill_rate_limit_for_source_backfill branch from c4b4976 to a28008b Compare November 19, 2024 07:43

xxchan force-pushed the 11-18-fix_fix_potential_data_loss_for_shared_source branch from 39887aa to 4469944 Compare November 19, 2024 07:59

xxchan force-pushed the 11-16-feat_support_backfill_rate_limit_for_source_backfill branch from a28008b to 9004675 Compare November 19, 2024 07:59

xxchan force-pushed the 11-18-fix_fix_potential_data_loss_for_shared_source branch from 4469944 to cf75bff Compare November 19, 2024 08:41

xxchan force-pushed the 11-16-feat_support_backfill_rate_limit_for_source_backfill branch from 9004675 to bf1c884 Compare November 19, 2024 08:41

xxchan requested review from tabVersion, kwannoel and stdrc November 19, 2024 09:16

kwannoel approved these changes Nov 19, 2024

View reviewed changes

kwannoel added the user-facing-changes Contains changes that are visible to users label Nov 19, 2024

tabVersion approved these changes Nov 19, 2024

View reviewed changes

xxchan force-pushed the 11-18-fix_fix_potential_data_loss_for_shared_source branch from cf75bff to 2ff1259 Compare November 20, 2024 02:09

xxchan force-pushed the 11-16-feat_support_backfill_rate_limit_for_source_backfill branch from bf1c884 to 3993c63 Compare November 20, 2024 02:09

xxchan mentioned this pull request Nov 20, 2024

feat: add rw_rate_limit system catalog #19466

Merged

9 tasks

xxchan changed the base branch from 11-18-fix_fix_potential_data_loss_for_shared_source to graphite-base/19445 November 20, 2024 07:58

xxchan mentioned this pull request Nov 20, 2024

cherry-pick: shared source data loss (#19443) and rate limit (#19445, #19466) #19482

Merged

9 tasks

feat: support backfill_rate_limit for source backfill

a7a89ea

xxchan force-pushed the 11-16-feat_support_backfill_rate_limit_for_source_backfill branch from 3993c63 to a7a89ea Compare November 20, 2024 10:52

xxchan force-pushed the graphite-base/19445 branch from 2ff1259 to b9c3f70 Compare November 20, 2024 10:52

xxchan changed the base branch from graphite-base/19445 to main November 20, 2024 10:53

graphite-app bot requested a review from a team November 20, 2024 11:33

xxchan added this pull request to the merge queue Nov 20, 2024

Merged via the queue into main with commit fe69ce3 Nov 20, 2024
32 of 33 checks passed

xxchan deleted the 11-16-feat_support_backfill_rate_limit_for_source_backfill branch November 20, 2024 11:41

neverchanje mentioned this pull request Nov 20, 2024

Document: feat: support backfill_rate_limit for source backfill risingwavelabs/risingwave-docs#2763

Open

github-merge-queue bot pushed a commit that referenced this pull request Nov 20, 2024

cherry-pick: shared source data loss (#19443) and rate limit (#19445, #…

79dbb2c

…19466) (#19482) Signed-off-by: xxchan <[email protected]> Co-authored-by: Noel Kwan <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support backfill_rate_limit for source backfill #19445

feat: support backfill_rate_limit for source backfill #19445

xxchan commented Nov 19, 2024 •

edited

Loading

xxchan commented Nov 19, 2024 •

edited

Loading

xxchan Nov 19, 2024

tabVersion Nov 19, 2024

tabVersion commented Nov 19, 2024

xxchan commented Nov 19, 2024

kwannoel left a comment

tabVersion left a comment

tabVersion Nov 19, 2024

xxchan Nov 19, 2024

tabVersion Nov 19, 2024

xxchan Nov 19, 2024

tabVersion Nov 19, 2024

xxchan commented Nov 20, 2024 •

edited

Loading

feat: support backfill_rate_limit for source backfill #19445

feat: support backfill_rate_limit for source backfill #19445

Conversation

xxchan commented Nov 19, 2024 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Release note

xxchan commented Nov 19, 2024 • edited Loading

xxchan Nov 19, 2024

Choose a reason for hiding this comment

tabVersion Nov 19, 2024

Choose a reason for hiding this comment

tabVersion commented Nov 19, 2024

xxchan commented Nov 19, 2024

kwannoel left a comment

Choose a reason for hiding this comment

tabVersion left a comment

Choose a reason for hiding this comment

tabVersion Nov 19, 2024

Choose a reason for hiding this comment

xxchan Nov 19, 2024

Choose a reason for hiding this comment

tabVersion Nov 19, 2024

Choose a reason for hiding this comment

xxchan Nov 19, 2024

Choose a reason for hiding this comment

tabVersion Nov 19, 2024

Choose a reason for hiding this comment

xxchan commented Nov 20, 2024 • edited Loading

Merge activity

xxchan commented Nov 19, 2024 •

edited

Loading

xxchan commented Nov 19, 2024 •

edited

Loading

xxchan commented Nov 20, 2024 •

edited

Loading