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

feat(stream): backfill resuming for sink #13665

Merged
merged 17 commits into from Nov 29, 2023
Merged

feat(stream): backfill resuming for sink #13665

merged 17 commits into from Nov 29, 2023

Conversation

kwannoel
Copy link
Contributor

@kwannoel kwannoel commented Nov 27, 2023

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

What's changed and what's your intention?

Closes #12080.

  • Recovery of the backfill progress (rw_ddl_progress) is unsupported currently. I will work on that separately.
  • The backfill progress tracking is still visible if cluster did not restart.
  • This PR makes sure that sink is recoverable even if backfill fails.
  • This PR makes sure sink is immediately visible after first barrier, independent of whether backfill is complete.

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

SINK DDL statements no longer need to wait for backfill to complete.

@kwannoel kwannoel marked this pull request as ready for review November 28, 2023 07:06
@kwannoel kwannoel requested review from BugenZhao, yezizp2012 and chenzl25 and removed request for BugenZhao and yezizp2012 November 28, 2023 07:15
Copy link
Contributor

@chenzl25 chenzl25 left a comment

Choose a reason for hiding this comment

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

LGTM! Maybe we should add a user-facing tag.

@kwannoel kwannoel added the user-facing-changes Contains changes that are visible to users label Nov 28, 2023
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Attention: 41 lines in your changes are missing coverage. Please review.

Comparison is base (b196953) 68.31% compared to head (c2e5099) 68.31%.
Report is 8 commits behind head on main.

Files Patch % Lines
src/meta/src/barrier/progress.rs 0.00% 28 Missing ⚠️
src/meta/src/manager/streaming_job.rs 35.71% 9 Missing ⚠️
src/meta/src/barrier/mod.rs 25.00% 3 Missing ⚠️
src/meta/src/rpc/ddl_controller.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13665      +/-   ##
==========================================
- Coverage   68.31%   68.31%   -0.01%     
==========================================
  Files        1523     1523              
  Lines      261833   261872      +39     
==========================================
+ Hits       178878   178885       +7     
- Misses      82955    82987      +32     
Flag Coverage Δ
rust 68.31% <16.32%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kwannoel kwannoel added this pull request to the merge queue Nov 28, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 28, 2023
@kwannoel kwannoel added this pull request to the merge queue Nov 28, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 28, 2023
@kwannoel kwannoel added this pull request to the merge queue Nov 28, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Nov 28, 2023
Copy link
Contributor

@yezizp2012 yezizp2012 left a comment

Choose a reason for hiding this comment

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

LGTM

@kwannoel kwannoel added this pull request to the merge queue Nov 29, 2023
Merged via the queue into main with commit 037b9a2 Nov 29, 2023
27 of 28 checks passed
@kwannoel kwannoel deleted the kwannoel/sink-return branch November 29, 2023 10:13
Comment on lines +156 to +157
/// 3. `Immediate`. Immediate jobs will immediately return,
/// but still register the lag between the job and its upstream.
Copy link
Member

Choose a reason for hiding this comment

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

Didn't find this variant? 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in https://github.com/risingwavelabs/risingwave/pull/13735/files, thanks!

It was used previously to indicate a new variant of stream job handling, but I realize I can just use a TrackingCommand without notifiers to accomplish the same thing, with less refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/run-backfill-tests type/feature user-facing-changes Contains changes that are visible to users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: backfill resuming for sink
4 participants