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: support multiple sinks into table #13659

Merged
merged 2 commits into from Dec 15, 2023
Merged

feat: support multiple sinks into table #13659

merged 2 commits into from Dec 15, 2023

Conversation

shanicky
Copy link
Contributor

@shanicky shanicky commented Nov 27, 2023

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

What's changed and what's your intention?

For creation, we will re-create the table with n additional merges and connect it to the existing (obtained through TableFragments from fragment manager) and newly built sink’s dispatcher in the meta node.

For dropping, we will re-create the table with n-1 additional merges on the meta node, connect it only with the dispatchers of the other sinks.

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.
  • All checks passed in ./risedev check (or alias, ./risedev c)

for input in &mut node.input {
if let Some(NodeBody::Merge(merge_node)) = &mut input.node_body && merge_node.upstream_actor_id.is_empty() {
if let Some(sink_id) = sink_id {
input.identity = format!("MergeExecutor(from sink {})", sink_id);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can assign identity of Merger at here cc @st1page

@shanicky shanicky linked an issue Dec 5, 2023 that may be closed by this pull request
@shanicky shanicky force-pushed the peng/multi-sink-into branch 2 times, most recently from 27f3117 to 84af28f Compare December 7, 2023 17:21
Copy link

codecov bot commented Dec 7, 2023

Codecov Report

Attention: 154 lines in your changes are missing coverage. Please review.

Comparison is base (c74817f) 68.06% compared to head (2390232) 68.03%.

Files Patch % Lines
src/meta/src/rpc/ddl_controller.rs 0.00% 113 Missing ⚠️
src/common/src/util/stream_graph_visitor.rs 0.00% 18 Missing ⚠️
src/frontend/src/handler/drop_sink.rs 0.00% 10 Missing ⚠️
src/meta/src/manager/catalog/mod.rs 0.00% 8 Missing ⚠️
src/frontend/src/handler/create_sink.rs 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13659      +/-   ##
==========================================
- Coverage   68.06%   68.03%   -0.04%     
==========================================
  Files        1535     1535              
  Lines      264826   264925      +99     
==========================================
- Hits       180266   180234      -32     
- Misses      84560    84691     +131     
Flag Coverage Δ
rust 68.03% <0.00%> (-0.04%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Base automatically changed from peng/sink-into-table-new to main December 9, 2023 16:34
@shanicky shanicky marked this pull request as ready for review December 9, 2023 17:14
…oming sinks in handle_create_sink

Implement e2e test for error messages, sink creation, table alteration, query sorting, sink and table dropping.

Code changes: Imports, function rename, argument modifications, variable rename, new arguments.

fix conflict

tmp

Signed-off-by: Shanicky Chen <peng@risingwave-labs.com>
Signed-off-by: Shanicky Chen <peng@risingwave-labs.com>
Copy link
Contributor

@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.

LGTM

@shanicky shanicky added this pull request to the merge queue Dec 15, 2023
Merged via the queue into main with commit 2633fbd Dec 15, 2023
26 of 27 checks passed
@shanicky shanicky deleted the peng/multi-sink-into branch December 15, 2023 08:03
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.

support multiple sink into table
3 participants