feat(frontend): support streaming_parallelism session variable. #7370

zwang28 · 2023-01-13T06:48:15Z

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

What's changed and what's your intention?

Implement feat: use session variable to control the parallelism of a streaming query #7359. streaming_parallelism precedes default parallelism.
Remove minimal_scheduling, because streaming_parallelism is more flexible to use.

The streaming_parallelism session var takes effect for

DdlServiceImpl::create_materialized_view
DdlServiceImpl::create_table
DdlServiceImpl::create_index
DdlServiceImpl::create_sink

TBD:

Any session var name better than streaming_parallelism?

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
~~- [ ] I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features).~~
All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

If your pull request contains user-facing changes, please specify the types of the changes, and create a release note. Otherwise, please feel free to remove this section.

Types of user-facing changes

Please keep the types that apply to your changes, and remove those that do not apply.

SQL commands, functions, and operators

Release note

Support streaming_parallelism session variable. It's not set by default, which results in using default parallelism of the cluster.

Refer to a related PR or issue link (optional)

#7359

codecov · 2023-01-13T07:13:35Z

Codecov Report

Merging #7370 (59554ad) into main (9fd7845) will increase coverage by 0.00%.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##             main    #7370   +/-   ##
=======================================
  Coverage   72.92%   72.92%           
=======================================
  Files        1071     1071           
  Lines      172382   172418   +36     
=======================================
+ Hits       125711   125739   +28     
- Misses      46671    46679    +8

Flag	Coverage Δ
rust	`72.92% <66.66%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
src/common/src/config.rs	`68.58% <ø> (ø)`
src/frontend/src/handler/create_table_as.rs	`0.00% <0.00%> (ø)`
src/meta/src/lib.rs	`2.40% <ø> (+0.02%)`	⬆️
src/meta/src/manager/env.rs	`95.72% <ø> (-0.04%)`	⬇️
src/meta/src/rpc/service/ddl_service.rs	`3.07% <0.00%> (+<0.01%)`	⬆️
src/common/src/session_config/mod.rs	`27.27% <33.33%> (+0.32%)`	⬆️
src/frontend/src/handler/alter_table.rs	`85.40% <100.00%> (+0.32%)`	⬆️
src/frontend/src/handler/create_index.rs	`88.01% <100.00%> (+0.11%)`	⬆️
src/frontend/src/handler/create_mv.rs	`94.95% <100.00%> (+0.07%)`	⬆️
src/frontend/src/handler/create_sink.rs	`97.20% <100.00%> (+0.10%)`	⬆️
... and 13 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

BugenZhao · 2023-01-13T09:03:25Z

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

-        let default_parallelism = if self.env.opts.minimal_scheduling {
+        let default_parallelism = if let Some(parallelism) = parallelism {
+            parallelism as usize
+        } else if self.env.opts.minimal_scheduling {


I guess it's time to remove this option, as it can be achieved by setting the variable, which sounds more flexible.

BugenZhao · 2023-01-13T09:04:43Z

proto/ddl_service.proto

@@ -88,6 +88,7 @@ message DropSinkResponse {
 message CreateMaterializedViewRequest {
  catalog.Table materialized_view = 1;
  stream_plan.StreamFragmentGraph fragment_graph = 2;
+  uint64 parallelism = 3;


Is it okay to put it into FragmentGraph? As it only makes sense for the streaming jobs, that is, with a graph in the DDL request. Seems this graph is not persisted and only used for the communication.

Make sense. Fixed.

# Conflicts: # dashboard/proto/gen/stream_plan.ts # proto/stream_plan.proto # src/frontend/src/handler/create_mv.rs

BugenZhao · 2023-01-16T06:09:45Z

proto/stream_plan.proto

@@ -617,4 +622,6 @@ message StreamFragmentGraph {
  repeated uint32 dependent_table_ids = 3;
  uint32 table_ids_cnt = 4;
  StreamEnvironment env = 5;
+  // 0 means use default value.
+  uint64 parallelism = 6;


Should we use the Parallelism message here? 👀

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

BugenZhao

LGTM 🚀 Thanks for the work!

fuyufjh · 2023-01-27T05:59:10Z

Should the batch query jobs i.e. SELECT follow this parallelism?

BugenZhao · 2023-01-27T06:10:39Z

Should the batch query jobs i.e. SELECT follow this parallelism?

Do you mean default parallelism of the non-scan stages? Good catch. 🤔

fuyufjh · 2023-01-27T06:18:19Z

Should the batch query jobs i.e. SELECT follow this parallelism?

Do you mean default parallelism of the non-scan stages? Good catch. 🤔

Yes, exactly.

fuyufjh · 2023-01-27T06:18:39Z

BTW, another example is CREATE TABLE. When creating a table, the underlying streaming job (for doing DMLs) will be created as well. Will streaming_parallelism be applied to it?

zwang28 · 2023-01-28T02:53:51Z

BTW, another example is CREATE TABLE. When creating a table, the underlying streaming job (for doing DMLs) will be created as well. Will streaming_parallelism be applied to it?

Yes.

BugenZhao · 2023-01-28T04:35:23Z

Should the batch query jobs i.e. SELECT follow this parallelism?

Do you mean default parallelism of the non-scan stages? Good catch. 🤔

Yes, exactly.

Maybe we need another BATCH_PARALLELISM for this. 😄

feat(frontend): support streaming_parallelism session variable.

bf8884a

github-actions bot added the type/feature label Jan 13, 2023

gen proto

2fccc52

add tests

fb35a4e

zwang28 requested review from BugenZhao, xxchan and jon-chuang January 13, 2023 08:44

BugenZhao reviewed Jan 13, 2023

View reviewed changes

zwang28 added 4 commits January 14, 2023 11:09

refactor: store parallelism in StreamFragmentGraph

9bbacc8

remove minimal_scheduling

3f27fd2

Merge branch 'main' into wangzheng/mv_parallelism

89bb7bb

# Conflicts: # dashboard/proto/gen/stream_plan.ts # proto/stream_plan.proto # src/frontend/src/handler/create_mv.rs

remove stale comment

a40598b

lmatz approved these changes Jan 16, 2023

View reviewed changes

lmatz added the mergify/can-merge label Jan 16, 2023

BugenZhao reviewed Jan 16, 2023

View reviewed changes

BugenZhao removed the mergify/can-merge label Jan 16, 2023

zwang28 added 3 commits January 16, 2023 22:49

refactor: use Parallelism proto message

381181f

Merge branch 'main' into wangzheng/mv_parallelism

a91a12c

gen-proto

9fcdb29

jon-chuang reviewed Jan 17, 2023

View reviewed changes

src/meta/src/rpc/service/ddl_service.rs Show resolved Hide resolved

BugenZhao approved these changes Jan 17, 2023

View reviewed changes

zwang28 added the mergify/can-merge label Jan 17, 2023

zwang28 and others added 3 commits January 17, 2023 11:56

Merge branch 'main' into wangzheng/mv_parallelism

039d37d

reduce log in e2e test

90bda52

Merge branch 'main' into wangzheng/mv_parallelism

59554ad

mergify bot merged commit 76d9dbc into main Jan 17, 2023

mergify bot deleted the wangzheng/mv_parallelism branch January 17, 2023 05:20

lmatz added the user-facing-changes Contains changes that are visible to users label Jan 17, 2023

fuyufjh mentioned this pull request Jan 27, 2023

RFC: System Parameters on Meta risingwavelabs/rfcs#34

Merged

fuyufjh mentioned this pull request Jan 28, 2023

feat(batch): support BATCH_PARALLELISM #7558

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(frontend): support streaming_parallelism session variable. #7370

feat(frontend): support streaming_parallelism session variable. #7370

zwang28 commented Jan 13, 2023 •

edited

Loading

codecov bot commented Jan 13, 2023 •

edited

Loading

BugenZhao Jan 13, 2023

zwang28 Jan 14, 2023

BugenZhao Jan 13, 2023

zwang28 Jan 14, 2023

BugenZhao Jan 16, 2023

BugenZhao left a comment

fuyufjh commented Jan 27, 2023

BugenZhao commented Jan 27, 2023

fuyufjh commented Jan 27, 2023

fuyufjh commented Jan 27, 2023

zwang28 commented Jan 28, 2023

BugenZhao commented Jan 28, 2023

feat(frontend): support streaming_parallelism session variable. #7370

feat(frontend): support streaming_parallelism session variable. #7370

Conversation

zwang28 commented Jan 13, 2023 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Types of user-facing changes

Release note

Refer to a related PR or issue link (optional)

codecov bot commented Jan 13, 2023 • edited Loading

Codecov Report

BugenZhao Jan 13, 2023

Choose a reason for hiding this comment

zwang28 Jan 14, 2023

Choose a reason for hiding this comment

BugenZhao Jan 13, 2023

Choose a reason for hiding this comment

zwang28 Jan 14, 2023

Choose a reason for hiding this comment

BugenZhao Jan 16, 2023

Choose a reason for hiding this comment

BugenZhao left a comment

Choose a reason for hiding this comment

fuyufjh commented Jan 27, 2023

BugenZhao commented Jan 27, 2023

fuyufjh commented Jan 27, 2023

fuyufjh commented Jan 27, 2023

zwang28 commented Jan 28, 2023

BugenZhao commented Jan 28, 2023

zwang28 commented Jan 13, 2023 •

edited

Loading

codecov bot commented Jan 13, 2023 •

edited

Loading