-
Notifications
You must be signed in to change notification settings - Fork 596
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
fix(stream): arrangement backfill background ddl #14563
Conversation
@@ -450,54 +451,58 @@ where | |||
"backfill_finished_wait_for_barrier" | |||
); | |||
// If not finished then we need to update state, otherwise no need. | |||
if let Message::Barrier(barrier) = &msg | |||
&& !is_completely_finished |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously we wait for this flag to pass. However is_completely_finished
could be true on recovery, if backfill is completed already.
In that case, we should just yield the barrier and stop waiting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, after recovery, the backfill state may be inconsistent with the CreateMaterializedTracker
state, so we need to notify the progress to finish every time after recovery.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we will always notify see https://github.com/risingwavelabs/risingwave/pull/14563/files#r1464421419. It is outside of the if-else block.
|
||
self.progress | ||
.finish(barrier.epoch.curr, total_snapshot_processed_rows); | ||
tracing::trace!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always notifies.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Fixes 2 bugs:
Adds background ddl + arrangement backfill as a test step in
main-cron.yml
.Reduces the logs being uploaded by:
Checklist
./risedev check
(or alias,./risedev c
)Documentation
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.