Skip to content

feat(streaming): remote channel building for mv on mv #352

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Feb 17, 2022
Merged

feat(streaming): remote channel building for mv on mv #352

merged 7 commits into from
Feb 17, 2022

Conversation

@MrCroxx
Copy link
Contributor

@MrCroxx MrCroxx commented Feb 16, 2022 •
edited
Loading

What's changed and what's your intention?

Changes:

  • Move all StreamGraph walk through to fragmenter.
  • Fix building remote channel for mv on mv.

Current state:

  • Most streaming e2e with 3node can run in local env.

TODO(in other prs):

  • Clean code in fragmenter.
  • Use row sort sqllogictest in e2e tests and enable 3node ci for streaming.

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests

Refer to a related PR or issue link (optional)

#324
#312

wyhyhyhyh
wyhyhyhyh reviewed Feb 16, 2022
View reviewed changes
rust/meta/src/stream/fragmenter.rs Outdated
Copy link
Contributor

@wyhyhyhyh wyhyhyhyh Feb 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

What is the difference between Graph and StreamGraph? Can you combine them? C.C @yezizp2012

Copy link
Contributor Author

@MrCroxx MrCroxx Feb 16, 2022

Choose a reason for hiding this comment

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

I'll refine fragmenter code after channel building is okay. It's kind of mess now.

@MrCroxx MrCroxx force-pushed the xx/distributed-channel branch from 96c331c to 56d46c0 Compare February 16, 2022 07:59
@MrCroxx MrCroxx mentioned this pull request Feb 16, 2022
Closed
3 tasks
@MrCroxx MrCroxx changed the title feat(streaming): remote channel building for mv on mv [WIP] feat(streaming): remote channel building for mv on mv Feb 16, 2022
@github-actions github-actions bot added the type/feature Type: New feature. label Feb 16, 2022
@MrCroxx MrCroxx marked this pull request as ready for review February 16, 2022 14:49
@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #352 (218ac77) into main (b9ccc17) will decrease coverage by 0.07%.
The diff coverage is 67.94%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #352      +/-   ##
============================================
- Coverage     74.25%   74.17%   -0.08%     
  Complexity     2681     2681              
============================================
  Files           863      864       +1     
  Lines         48880    49114     +234     
  Branches       1591     1591              
============================================
+ Hits          36295    36430     +135     
- Misses        11772    11871      +99     
  Partials        813      813
Flag Coverage Δ
java 62.00% <ø> (ø)
rust 79.36% <67.94%> (-0.15%) ⬇️

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

Impacted Files Coverage Δ
rust/meta/src/manager/stream_clients.rs 80.00% <0.00%> (-12.31%) ⬇️
rust/meta/src/rpc/service/stream_service.rs 100.00% <ø> (ø)
rust/server/src/rpc/service/stream_service.rs 9.09% <ø> (ø)
rust/stream/src/task/stream_manager.rs 51.48% <25.00%> (-1.35%) ⬇️
rust/meta/src/stream/stream_manager.rs 80.19% <27.58%> (-6.47%) ⬇️
rust/meta/src/stream/graph/stream_graph.rs 56.52% <28.20%> (-27.63%) ⬇️
rust/meta/src/model/stream.rs 62.50% <33.33%> (-1.79%) ⬇️
rust/meta/src/stream/fragmenter.rs 85.71% <83.33%> (-1.39%) ⬇️
rust/meta/src/stream/test_fragmenter.rs 98.71% <100.00%> (+0.05%) ⬆️
rust/stream/src/executor/dispatch.rs 85.63% <100.00%> (-0.13%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9ccc17...218ac77. Read the comment docs.

wyhyhyhyh
wyhyhyhyh approved these changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@wyhyhyhyh wyhyhyhyh left a comment

Choose a reason for hiding this comment

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

LSTM if it works.
Need to refactor some messy implementation later.

@MrCroxx MrCroxx merged commit c3880d1 into main Feb 17, 2022
@MrCroxx MrCroxx deleted the xx/distributed-channel branch February 17, 2022 02:50
This was referenced Feb 17, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@wyhyhyhyh wyhyhyhyh wyhyhyhyh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type/feature Type: New feature.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MrCroxx @wyhyhyhyh