feat(frontend): auto disable snapshot backfill in some cases #24360
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wenym1
force-pushed
the
yiming/auto-disable-snapshot-backfill
branch
from
c2adadc to
391b731
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR automatically disables snapshot backfill in cases where it's not supported or could cause issues, specifically for temporal joins and shared sources. The implementation adds an is_shared property to logical source nodes and introduces a visitor pattern to detect when snapshot backfill should be disabled, providing user notifications in these scenarios.
- Adds
forbid_snapshot_backfillmethod that uses a visitor pattern to detect temporal joins and shared sources - Updates
should_use_snapshot_backfillto check and disable snapshot backfill with user notification when forbidden - Introduces
is_shared_sourcehelper method and exposesis_sharedproperty in logical plan output
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/frontend/src/optimizer/plan_node/mod.rs |
Adds forbid_snapshot_backfill method with visitor pattern to detect temporal joins and shared sources; imports required visitor types |
src/frontend/src/optimizer/plan_node/logical_source.rs |
Adds is_shared_source helper method; updates Distill implementation to display is_shared property in plan output |
src/frontend/src/optimizer/plan_node/logical_join.rs |
Adds should_be_temporal_join public helper method to check if join should be treated as temporal |
src/frontend/src/optimizer/mod.rs |
Updates should_use_snapshot_backfill to call forbid_snapshot_backfill and notify users when snapshot backfill is auto-disabled |
src/frontend/planner_test/tests/testdata/output/watermark.yaml |
Updates test output to include is_shared: false property in logical source |
src/frontend/planner_test/tests/testdata/output/tpch_variant.yaml |
Updates all logical source nodes in test output to include is_shared: false property |
src/frontend/planner_test/tests/testdata/output/temporal_join.yaml |
Changes expected output from error to valid stream plan showing arrangement backfill is used when snapshot backfill is disabled |
src/frontend/planner_test/tests/testdata/output/subquery.yaml |
Updates logical source nodes to include is_shared: false property |
src/frontend/planner_test/tests/testdata/output/shared_source.yml |
Adds new test case demonstrating snapshot backfill is disabled with shared sources |
src/frontend/planner_test/tests/testdata/output/share.yaml |
Updates logical source nodes to include is_shared: false property |
src/frontend/planner_test/tests/testdata/output/nexmark_watermark.yaml |
Updates all nexmark logical source nodes to include is_shared: false property |
src/frontend/planner_test/tests/testdata/output/nexmark_source_kafka.yaml |
Updates all nexmark Kafka source nodes to include is_shared: true property |
src/frontend/planner_test/tests/testdata/output/nexmark_source.yaml |
Updates all nexmark source nodes to include is_shared: false property |
src/frontend/planner_test/tests/testdata/output/batch_source.yaml |
Updates logical source node to include is_shared: true property |
src/frontend/planner_test/tests/testdata/input/temporal_join.yaml |
Changes expected output type from stream_error to stream_plan for temporal join test |
src/frontend/planner_test/tests/testdata/input/shared_source.yml |
Adds new test configuration with snapshot backfill enabled to verify auto-disable behavior |
e2e_test/source_inline/kafka/shared_source.slt.serial |
Removes obsolete test case that expected error for snapshot backfill with shared sources |
tabVersion
reviewed
Jan 7, 2026
Comment on lines
-30
to
-31
| statement error Not supported: Snapshot backfill with shared source backfill is not supported | ||
| create materialized view mv_snapshot_backfill_shared_source as (select * from mv_before_produce) union (select * from s_before_produce); |
Contributor
There was a problem hiding this comment.
what is the test case's expect behavior now?
Contributor
Author
There was a problem hiding this comment.
After this PR, even though snapshot backfill is enabled by session variable, it will choose to use arrangement backfill anyway with a notice to user about the implicit switch.
Comment on lines
+568
to
+571
| struct ForbidSnapshotBackfill { | ||
| warning_msg: Option<String>, | ||
| } | ||
| impl LogicalPlanVisitor for ForbidSnapshotBackfill { |
Contributor
There was a problem hiding this comment.
any reason to def a struct inside a function?
Contributor
Author
There was a problem hiding this comment.
This struct is a helper struct only used in this method to leverage the visitor trait. Defined in the function to limit its visibility.
wenym1
force-pushed
the
yiming/auto-disable-snapshot-backfill
branch
from
391b731 to
2492744
Compare
tabVersion
approved these changes
Jan 8, 2026
Contributor
tabVersion
left a comment
tabVersion
left a comment
There was a problem hiding this comment.
LGTM from source side, not sure if the backfill strategy switch has other effects.
cc streaming experts @chenzl25 @BugenZhao
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This pull request updates planner test outputs and test cases to reflect the addition of an
is_sharedproperty to logical source nodes, and adjusts the expected planner behavior for temporal joins and shared source configurations. The main changes are to test data and expected outputs, ensuring that test cases now properly account for whether a source is shared or not, and that planner output matches the new logical plan structure.Test output updates for
is_sharedproperty:is_sharedfield toLogicalSourcenodes in test output files, marking sources as eithertrueorfalsedepending on whether they are shared. This affects multiple files, includingbatch_source.yaml,nexmark_source.yaml,nexmark_source_kafka.yaml, andnexmark_watermark.yaml. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]Test case and configuration changes:
shared_source.ymltest input to include a configuration map enablingstreaming_use_shared_source, and added a scenario withstreaming_use_snapshot_backfillenabled.Planner behavior adjustments:
Checklist
Documentation
Release note