fix(frontend): create mview on singleton upstream mview #4153

BugenZhao · 2022-07-25T12:24:41Z

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

This PR fixes that we cannot create an mview on a singleton upstream mview. There're several reasons.

StreamTableScan does not check whether the distribution key of upstream mview is empty, and always use HashShard distribution.
If there's no exchange in the stream graph of new mview, we'll use the default is_singleton = false regardless of the distribution.
The scheduler currently assume that the downstream chain actors lie on the same parallel unit of the upstream materializes, which makes it fail to resolve the upstream actor id of singleton mview.
We won't broadcast the downstream chain actor infos to the upstream worker. When it creates a Output, it needs the downstream address to decide whether we need a RemoteOutput.

This PR also includes some style refinement of the stream manager. 💦

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
All checks passed in ./risedev check (or alias, ./risedev c)

Refer to a related PR or issue link (optional)

Fix cannot create MV over simple agg MV #4067

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

st1page

LGTM

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

BugenZhao · 2022-07-26T05:11:38Z

src/frontend/src/optimizer/plan_node/batch_seq_scan.rs

@@ -78,6 +79,11 @@ impl BatchSeqScan {
                Distribution::Single
            } else {
                match self.logical.distribution_key() {
+                    // FIXME: Should be `Single` if no distribution key.


Opened #4164 for this.

BugenZhao · 2022-07-26T05:18:08Z

src/frontend/src/optimizer/property/distribution.rs

+                    assert!(
+                        !key.is_empty(),
+                        "hash key should not be empty, use `Single` instead"
+                    );


Opened #4165 for this.

BowenXiao1999

LGTM

BowenXiao1999 · 2022-07-26T05:13:59Z

proto/stream_plan.proto

+  // fragment in the downstream mview. Remove this when we refactor the fragmenter.
+  bool is_singleton = 6;


What is the desired way?

We may directly operate on the Fragment struct in the frontend instead of the protobuf struct, so that we may be able to directly find whether a fragment is a singleton or not instead of checking the dispatcher type.

Currently, we cannot check whether is_singleton for the most upstream fragment since there's no exchange before it. That's why we need hacking for TopN and Chain. 😢

cc @st1page

src/frontend/src/stream_fragmenter/graph/fragment_graph.rs

BowenXiao1999 · 2022-07-26T05:20:27Z

src/frontend/src/stream_fragmenter/mod.rs

                // memorize table id for later use
                state
                    .dependent_table_ids
                    .insert(TableId::new(node.table_id));
+                current_fragment.is_singleton = node.is_singleton;


IIUC, the flag node.is_singleton for chain_node is alwasy true? Or it depends on the whether the upstream mview is singleton or not.

Or it depends on the whether the upstream mview is singleton or not.

✔️

BowenXiao1999 · 2022-07-26T05:23:51Z

src/meta/src/rpc/service/ddl_service.rs

@@ -467,6 +467,7 @@ where
        for fragment in fragment_graph.fragments.values_mut() {
            mview_count += fill_mview_id(fragment.node.as_mut().unwrap(), mview_id);
        }
+        let fragment_graph = fragment_graph;


Suggested change

let fragment_graph = fragment_graph;

This's for making it immutable, so we won't fear that the variable will unexpectedly change after this line. 🥺

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

…n-singleton

codecov · 2022-07-26T07:09:13Z

Codecov Report

Merging #4153 (99832bf) into main (27d203c) will increase coverage by 0.03%.
The diff coverage is 69.33%.

@@            Coverage Diff             @@
##             main    #4153      +/-   ##
==========================================
+ Coverage   73.92%   73.96%   +0.03%     
==========================================
  Files         839      839              
  Lines      119199   119202       +3     
==========================================
+ Hits        88121    88165      +44     
+ Misses      31078    31037      -41

Flag	Coverage Δ
rust	`73.96% <69.33%> (+0.03%)`	⬆️

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

Impacted Files	Coverage Δ
...c/frontend/src/optimizer/plan_node/logical_scan.rs	`88.57% <ø> (ø)`
src/frontend/src/scheduler/local.rs	`0.00% <0.00%> (ø)`
src/meta/src/rpc/service/ddl_service.rs	`3.71% <0.00%> (+0.02%)`	⬆️
src/stream/src/task/stream_manager.rs	`2.98% <0.00%> (+0.01%)`	⬆️
src/frontend/src/stream_fragmenter/mod.rs	`91.53% <40.00%> (+0.76%)`	⬆️
...ntend/src/optimizer/plan_node/stream_index_scan.rs	`27.27% <50.00%> (-0.26%)`	⬇️
src/meta/src/stream/scheduler.rs	`91.57% <50.00%> (-0.33%)`	⬇️
src/meta/src/stream/stream_manager.rs	`76.81% <61.81%> (+0.53%)`	⬆️
src/stream/src/executor/exchange/output.rs	`50.87% <75.00%> (-0.04%)`	⬇️
...frontend/src/optimizer/plan_node/batch_seq_scan.rs	`93.10% <81.25%> (-0.74%)`	⬇️
... and 17 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

…bs#4153) * use singleton distribution on stream scan Signed-off-by: Bugen Zhao <i@bugenzhao.com> * refine parallelism allocation Signed-off-by: Bugen Zhao <i@bugenzhao.com> * fix the bug Signed-off-by: Bugen Zhao <i@bugenzhao.com> * fix local mode Signed-off-by: Bugen Zhao <i@bugenzhao.com> * refine docs Signed-off-by: Bugen Zhao <i@bugenzhao.com> * disable batch fix Signed-off-by: Bugen Zhao <i@bugenzhao.com> * add tests Signed-off-by: Bugen Zhao <i@bugenzhao.com> * fix tests Signed-off-by: Bugen Zhao <i@bugenzhao.com> * fix distributed e2e Signed-off-by: Bugen Zhao <i@bugenzhao.com> * remove logging Signed-off-by: Bugen Zhao <i@bugenzhao.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

BugenZhao added 3 commits July 25, 2022 17:47

use singleton distribution on stream scan

6128dd4

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

refine parallelism allocation

367c9d0

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

fix the bug

430e655

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

BugenZhao requested a review from st1page July 25, 2022 12:24

github-actions bot added the type/fix Bug fix label Jul 25, 2022

BugenZhao mentioned this pull request Jul 25, 2022

cannot create MV over simple agg MV #4067

Closed

st1page approved these changes Jul 26, 2022

View reviewed changes

BugenZhao added 4 commits July 26, 2022 10:55

fix local mode

f1c6f36

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

refine docs

74ae359

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

disable batch fix

fe4dcc4

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

add tests

95d3a1d

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

BugenZhao marked this pull request as ready for review July 26, 2022 04:06

BugenZhao requested review from xxchan and skyzh July 26, 2022 04:07

fix tests

eb8109f

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

BugenZhao requested a review from BowenXiao1999 July 26, 2022 04:54

This was referenced Jul 26, 2022

batch: singleton seq scan on local mode #4164

Closed

frontend: ensure dist key not empty in Distribution::HashShard #4165

Closed

BugenZhao commented Jul 26, 2022

View reviewed changes

BowenXiao1999 approved these changes Jul 26, 2022

View reviewed changes

BugenZhao added 3 commits July 26, 2022 14:43

fix distributed e2e

6b14c11

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

remove logging

b48cc6f

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

Merge remote-tracking branch 'origin/main' into bz/fix-create-mview-o…

fa82839

…n-singleton

BugenZhao added the mergify/can-merge label Jul 26, 2022

mergify bot added 2 commits July 26, 2022 07:09

Merge branch 'main' into bz/fix-create-mview-on-singleton

4f6f8d7

Merge branch 'main' into bz/fix-create-mview-on-singleton

99832bf

mergify bot merged commit ea29596 into main Jul 26, 2022

mergify bot deleted the bz/fix-create-mview-on-singleton branch July 26, 2022 07:35

CharlieSYH mentioned this pull request Aug 10, 2022

Update CREATE MATERIALIZED VIEW risingwavelabs/risingwave-docs#204

Closed

BugenZhao mentioned this pull request Aug 18, 2022

feat: notify actor info when CN joined and dynamically adjust the parallelism of chain #4710

Closed

3 tasks

CharlieSYH mentioned this pull request Aug 30, 2022

Adding an example section for CREATE MV risingwavelabs/risingwave-docs#247

Merged

4 tasks

BugenZhao mentioned this pull request Mar 14, 2023

refactor(fragmenter): remove is_singleton workarounds on Chain #8536

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(frontend): create mview on singleton upstream mview #4153

fix(frontend): create mview on singleton upstream mview #4153

BugenZhao commented Jul 25, 2022 •

edited

Loading

st1page left a comment

BugenZhao Jul 26, 2022

BugenZhao Jul 26, 2022

BowenXiao1999 left a comment

BowenXiao1999 Jul 26, 2022

BugenZhao Jul 26, 2022 •

edited

Loading

BowenXiao1999 Jul 26, 2022

BugenZhao Jul 26, 2022

BowenXiao1999 Jul 26, 2022

BugenZhao Jul 26, 2022

codecov bot commented Jul 26, 2022 •

edited

Loading

		// fragment in the downstream mview. Remove this when we refactor the fragmenter.
		bool is_singleton = 6;

fix(frontend): create mview on singleton upstream mview #4153

fix(frontend): create mview on singleton upstream mview #4153

Conversation

BugenZhao commented Jul 25, 2022 • edited Loading

What's changed and what's your intention?

Checklist

Refer to a related PR or issue link (optional)

st1page left a comment

Choose a reason for hiding this comment

BugenZhao Jul 26, 2022

Choose a reason for hiding this comment

BugenZhao Jul 26, 2022

Choose a reason for hiding this comment

BowenXiao1999 left a comment

Choose a reason for hiding this comment

BowenXiao1999 Jul 26, 2022

Choose a reason for hiding this comment

BugenZhao Jul 26, 2022 • edited Loading

Choose a reason for hiding this comment

BowenXiao1999 Jul 26, 2022

Choose a reason for hiding this comment

BugenZhao Jul 26, 2022

Choose a reason for hiding this comment

BowenXiao1999 Jul 26, 2022

Choose a reason for hiding this comment

BugenZhao Jul 26, 2022

Choose a reason for hiding this comment

codecov bot commented Jul 26, 2022 • edited Loading

Codecov Report

BugenZhao commented Jul 25, 2022 •

edited

Loading

BugenZhao Jul 26, 2022 •

edited

Loading

codecov bot commented Jul 26, 2022 •

edited

Loading