Skip to content
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

feat(meta): support schema change in actor graph builder #7878

Merged
merged 5 commits into from
Feb 16, 2023

Conversation

BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Feb 13, 2023

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

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

This PR extends the new actor graph builder introduced in #7659 for replacing table plan, and adds the procedure of preparing the TableFragments. Note that the RPC with the compute nodes is not implemented so this is not utilized by the frontend yet.

Building an actor graph for schema change can be similar and symmetric to the MV on MV.

  • For MV on MV, we have existing upstream Materialize fragments that give the requirements to the distribution of current Chain. We'll generate new dispatchers for the upstream Materialize in the result.
  • For replacing table plan for schema change, we (may) have existing downstream Chain fragments that give the requirements to the distribution of current Materialize. We'll generate updates for the merger for the downstream Chain in the result.

Related:

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).
  • I have demonstrated that backward compatibility is not broken by breaking changes and created issues to track deprecated features to be removed in the future. (Please refer to the issue)
  • All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

  • My PR DOES NOT contain user-facing changes.
Click here for Documentation

Types of user-facing changes

Please keep the types that apply to your changes, and remove the others.

  • Installation and deployment
  • Connector (sources & sinks)
  • SQL commands, functions, and operators
  • RisingWave cluster configuration changes
  • Other (please specify in the release note below)

Release note

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao changed the title feat(meta): build actor graph for schema change feat(meta): build actor graph for schema change [WIP] Feb 13, 2023
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
…-part-1

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao changed the title feat(meta): build actor graph for schema change [WIP] feat(meta): support schema change in actor graph builder Feb 14, 2023
@BugenZhao BugenZhao marked this pull request as ready for review February 14, 2023 06:38
// - For scaling, this is always `None`.
// - For plan change, the upstream fragment will be changed to a new one, and this will be `Some`.
// In this case, all the upstream actors should be removed and replaced by the `new` ones.
optional uint32 new_upstream_fragment_id = 5;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may reuse the Update barrier of scaling for schema change.

@@ -581,18 +587,12 @@ where

// 2. Get the env for streaming jobs.
let env = StreamEnvironment::from_protobuf(fragment_graph.get_env().unwrap());
let default_parallelism = if let Some(Parallelism { parallelism }) =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We put this into FragmentGraph::new.

@BugenZhao BugenZhao requested review from yezizp2012 and chenzl25 and removed request for yezizp2012 February 14, 2023 06:41
Copy link
Contributor

@chenzl25 chenzl25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! It's interesting to think of schema changes and mv on mv as dual problems in some sense. I like this idea.

Copy link
Member

@yezizp2012 yezizp2012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LSTM!

@mergify mergify bot merged commit 7f13a0a into main Feb 16, 2023
@mergify mergify bot deleted the bz/schema-change-meta-part-1 branch February 16, 2023 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants