-
Notifications
You must be signed in to change notification settings - Fork 590
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(dyn-filter): filter left changes according to watermark before writing state table #17816
Conversation
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
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.
LGTM
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.
LGTM, thanks!
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
This stack of pull requests is managed by Graphite. Learn more about stacking. |
ExprType::LessThan | ExprType::LessThanOrEqual => { | ||
// For <= and <, watermark on rhs means all rows older than the watermark should already be emitted, | ||
// and future lhs inputs should be directly passed to downstream without any state table operation. | ||
// So, the state table can be cleaned up. | ||
true | ||
} |
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.
Now the left table is cleaned by rhs watermark when comparator is <
and <=
as well.
// that has been applied to the state table, just like why we use `prev_epoch_value` | ||
// instead of `current_epoch_value` for `filter_condition`. | ||
let below_watermark_condition = | ||
build_cond(LessThan, self.left_table.get_committed_watermark().cloned()) |
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.
Thanks to the removal of "state clean watermark buffer", now we can get up-to-date committed watermark from state table (and even after recovery).
Signed-off-by: Richard Chien <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
After #17767, we can get persisted watermark value for each vnode in state table. This PR utilizes this feature to filter out changes that are below committed table watermark before writing into state table. This fixes #17711.
close #14014
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.