refactor(meta): simplify stream job table/source id assignment #19171

xxchan · 2024-10-29T07:56:48Z

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

What's changed and what's your intention?

We fill table_id/source_id in several places, which seems unnecessary. This PR tries to remove unnecessary codes to make it more understandable and maintainable.

We leave it with 2 steps:

In create_job_catalog, we assign id to StreamingJob
In StreamFragmentGraph::new (fill_job), we traverse the nodes, and set the id to corresponding plan nodes. (Note that previously we traverse the nodes multiple times)

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
I have added test labels as necessary. See details.
I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
All checks passed in ./risedev check (or alias, ./risedev c)
My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

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.

xxchan · 2024-10-29T07:56:59Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @xxchan and the rest of your teammates on Graphite

xxchan · 2024-10-29T15:17:09Z

src/meta/src/stream/stream_graph/fragment.rs

rename table -> job

…ub.com/risingwavelabs/risingwave/blob/193e93fd8d9f9dbae717fe6a5b411e7f33382f27/src/meta/src/controller/streaming_job.rs#L247-L251 - stream node: filled in fill_job Signed-off-by: xxchan <xxchan22f@gmail.com>

xxchan · 2024-11-04T06:01:34Z

src/meta/src/stream/stream_graph/fragment.rs

+            NodeBody::Source(source_node) => {
+                match job {
+                    // Note: For table without connector, it has a dumb Source node.
+                    // Note: For table with connector, it's source node has a source id different with the table id (job id), assigned in create_job_catalog.
+                    StreamingJob::Table(source, _table, _table_job_type) => {
+                        if let Some(source_inner) = source_node.source_inner.as_mut() {
+                            if let Some(source) = source {
+                                debug_assert_ne!(source.id, job_id);
+                                source_inner.source_id = source.id;
+                            }
+                        }
+                    }
+                    StreamingJob::Source(source) => {
+                        has_job = true;
+                        if let Some(source_inner) = source_node.source_inner.as_mut() {
+                            debug_assert_eq!(source.id, job_id);
+                            source_inner.source_id = source.id;
+                        }
+                    }
+                    // For other job types, no need to fill the source id, since it refers to an existing source.
+                    _ => {}
+                }
+            }
+            NodeBody::StreamCdcScan(node) => {
+                if let Some(table_desc) = node.cdc_table_desc.as_mut() {
+                    table_desc.table_id = job_id;


This replaces the code in create_streaming_job

xxchan · 2024-11-04T06:01:52Z

src/meta/src/rpc/ddl_controller.rs

-        match &mut streaming_job {
-            StreamingJob::Table(src, table, job_type) => {
-                // If we're creating a table with connector, we should additionally fill its ID first.
-                fill_table_stream_graph_info(src, table, *job_type, &mut fragment_graph);
-            }
-            StreamingJob::Source(src) => {
-                // set the inner source id of source node.
-                for fragment in fragment_graph.fragments.values_mut() {
-                    visit_fragment(fragment, |node_body| {
-                        if let NodeBody::Source(source_node) = node_body {
-                            source_node.source_inner.as_mut().unwrap().source_id = src.id;
-                        }
-                    });
-                }
-            }
-            _ => {}


do it in fill_job instead. (They already have some overlaps)

xxchan · 2024-11-04T06:14:28Z

src/meta/service/src/ddl_service.rs

-    fn extract_replace_table_info(change: ReplaceTablePlan) -> ReplaceTableInfo {
-        let job_type = change.get_job_type().unwrap_or_default();
-        let mut source = change.source;
-        let mut fragment_graph = change.fragment_graph.unwrap();
-        let mut table = change.table.unwrap();
-        if let Some(OptionalAssociatedSourceId::AssociatedSourceId(source_id)) =
-            table.optional_associated_source_id
-        {
-            source.as_mut().unwrap().id = source_id;
-            fill_table_stream_graph_info(&mut source, &mut table, job_type, &mut fragment_graph);
-        }
-        let table_col_index_mapping = change
-            .table_col_index_mapping


Note: extract_replace_table_info is used in: create/drop sink & table schema change.

Some work is done in fill_job, and some is done in frontend generate_stream_graph_for_replace_table

xxchan · 2024-11-04T06:16:10Z

src/frontend/src/handler/create_table.rs

+        source.as_mut().unwrap().optional_associated_table_id =
+            Some(OptionalAssociatedTableId::AssociatedTableId(table.id))


It's strange not to set optional_associated_table_id here.

Note: for replace table, the Source catalog here were both created again. It looks error-prone, but do not intend to change it.

Signed-off-by: xxchan <xxchan22f@gmail.com>

src/meta/src/stream/stream_graph/fragment.rs

stdrc

Appreciate the work done in this PR👍

Co-authored-by: Bugen Zhao <i@bugenzhao.com>

github-actions bot added the Invalid PR Title label Oct 29, 2024

xxchan mentioned this pull request Oct 29, 2024

refactor: move server.id for MySQL CDC from meta to fe #19169

Merged

9 tasks

github-actions bot added the ci/run-e2e-single-node-tests label Oct 29, 2024

xxchan marked this pull request as ready for review October 29, 2024 07:57

github-actions bot added type/refactor and removed Invalid PR Title labels Oct 29, 2024

xxchan force-pushed the xxchan/refactor-ddl branch from 0dab5fe to 016a1e5 Compare October 29, 2024 08:12

xxchan force-pushed the xxchan/desirable-trout branch from a568646 to 3f64c85 Compare October 29, 2024 08:17

xxchan force-pushed the xxchan/refactor-ddl branch from 016a1e5 to 2c7a812 Compare October 29, 2024 08:17

xxchan force-pushed the xxchan/desirable-trout branch from 3f64c85 to 0399933 Compare October 29, 2024 08:19

xxchan force-pushed the xxchan/refactor-ddl branch from 2c7a812 to 744d680 Compare October 29, 2024 08:19

xxchan force-pushed the xxchan/desirable-trout branch 2 times, most recently from fa620ec to 9f4c672 Compare October 29, 2024 14:23

xxchan force-pushed the xxchan/refactor-ddl branch from 744d680 to 2404113 Compare October 29, 2024 14:23

github-actions bot added the ci/run-e2e-test-other-backends label Oct 29, 2024

xxchan force-pushed the xxchan/desirable-trout branch from 9f4c672 to 2cab4a1 Compare October 29, 2024 15:11

xxchan force-pushed the xxchan/refactor-ddl branch from 2404113 to ae16bf3 Compare October 29, 2024 15:11

xxchan mentioned this pull request Oct 29, 2024

refactor: rename ConnectorSchema/SourceSchema to FormatEncode #19174

Merged

9 tasks

xxchan force-pushed the xxchan/refactor-ddl branch from ae16bf3 to 6de33ff Compare October 29, 2024 15:16

xxchan commented Oct 29, 2024

View reviewed changes

src/meta/src/stream/stream_graph/fragment.rs

Copy link

Member Author

xxchan Oct 29, 2024

Choose a reason for hiding this comment

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

rename table -> job

xxchan force-pushed the xxchan/desirable-trout branch from 2cab4a1 to f59bc27 Compare October 29, 2024 16:00

xxchan force-pushed the xxchan/refactor-ddl branch from 6de33ff to 0e24225 Compare October 29, 2024 16:01

xxchan changed the base branch from xxchan/desirable-trout to xxchan/format-encode October 29, 2024 16:01

Base automatically changed from xxchan/format-encode to main October 30, 2024 01:59

xxchan force-pushed the xxchan/refactor-ddl branch from 0e24225 to f33a42a Compare October 30, 2024 02:19

xxchan mentioned this pull request Oct 30, 2024

feat: support alter shared source #18935

Draft

9 tasks

xxchan force-pushed the xxchan/refactor-ddl branch 2 times, most recently from d43598b to 0012e27 Compare November 4, 2024 05:50

- table with connector: filled when creating job catalog https://gith…

613044f

…ub.com/risingwavelabs/risingwave/blob/193e93fd8d9f9dbae717fe6a5b411e7f33382f27/src/meta/src/controller/streaming_job.rs#L247-L251 - stream node: filled in fill_job Signed-off-by: xxchan <xxchan22f@gmail.com>

xxchan force-pushed the xxchan/refactor-ddl branch from 0012e27 to 613044f Compare November 4, 2024 05:59

xxchan commented Nov 4, 2024

View reviewed changes

fix

6eb4dd7

Signed-off-by: xxchan <xxchan22f@gmail.com>

xxchan requested review from yezizp2012 and BugenZhao November 4, 2024 06:51

xxchan changed the title ~~refactor(meta): simplify create stream job~~ refactor(meta): simplify stream job id assignment Nov 4, 2024

BugenZhao approved these changes Nov 4, 2024

View reviewed changes

src/meta/src/stream/stream_graph/fragment.rs Outdated Show resolved Hide resolved

yezizp2012 requested a review from shanicky November 4, 2024 06:59

xxchan changed the title ~~refactor(meta): simplify stream job id assignment~~ refactor(meta): simplify stream job table/source id assignment Nov 4, 2024

yezizp2012 approved these changes Nov 4, 2024

View reviewed changes

stdrc reviewed Nov 4, 2024

View reviewed changes

Update src/meta/src/stream/stream_graph/fragment.rs

6593d0c

Co-authored-by: Bugen Zhao <i@bugenzhao.com>

xxchan enabled auto-merge November 4, 2024 09:49

xxchan added this pull request to the merge queue Nov 4, 2024

Merged via the queue into main with commit a9f8945 Nov 4, 2024
33 of 34 checks passed

xxchan deleted the xxchan/refactor-ddl branch November 4, 2024 10:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(meta): simplify stream job table/source id assignment #19171

refactor(meta): simplify stream job table/source id assignment #19171

xxchan commented Oct 29, 2024 •

edited

Loading

xxchan commented Oct 29, 2024 •

edited

Loading

xxchan Oct 29, 2024

xxchan Nov 4, 2024

xxchan Nov 4, 2024 •

edited

Loading

xxchan Nov 4, 2024

xxchan Nov 4, 2024

xxchan Nov 4, 2024 •

edited

Loading

stdrc left a comment

		source.as_mut().unwrap().optional_associated_table_id =
		Some(OptionalAssociatedTableId::AssociatedTableId(table.id))

refactor(meta): simplify stream job table/source id assignment #19171

refactor(meta): simplify stream job table/source id assignment #19171

Conversation

xxchan commented Oct 29, 2024 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Release note

xxchan commented Oct 29, 2024 • edited Loading

xxchan Oct 29, 2024

Choose a reason for hiding this comment

xxchan Nov 4, 2024

Choose a reason for hiding this comment

xxchan Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

xxchan Nov 4, 2024

Choose a reason for hiding this comment

xxchan Nov 4, 2024

Choose a reason for hiding this comment

xxchan Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

stdrc left a comment

Choose a reason for hiding this comment

xxchan commented Oct 29, 2024 •

edited

Loading

xxchan commented Oct 29, 2024 •

edited

Loading

xxchan Nov 4, 2024 •

edited

Loading

xxchan Nov 4, 2024 •

edited

Loading