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(streaming): introduce new source executor #6447

Merged
merged 9 commits into from
Nov 21, 2022
Merged

Conversation

xx01cyx
Copy link
Contributor

@xx01cyx xx01cyx commented Nov 17, 2022

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

What's changed and what's your intention?

This is the first step of the new DML design.

Summarize your change

Introduce new source executor. It differs from the current one in following ways:

  1. The streaming source (SourceExecutorV2.stream_source_core) becomes optional. That is to say, it is possible that a new source executor has no external connector. Under this circumstance, the new source executor's only responsibility is to receive and forward barrier messages.
  2. The new source executor will not generate row id for data. This will be done in RowIdGenerateExecutor (to be implemented).

Describe any limitations of the current code

Datagen is used to mock external connector in UT now. A better approach will be introducing a mock connector only for testing. (#6427)

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Related Issue

#5949

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has totally checked 2356 files.

Valid Invalid Ignored Fixed
1139 1 1216 0
Click to see the invalid file list
  • src/stream/src/executor/source/source_executor_v2.rs

@codecov
Copy link

codecov bot commented Nov 17, 2022

Codecov Report

Merging #6447 (91ce031) into main (709e1be) will increase coverage by 0.04%.
The diff coverage is 82.89%.

@@            Coverage Diff             @@
##             main    #6447      +/-   ##
==========================================
+ Coverage   73.94%   73.98%   +0.04%     
==========================================
  Files         981      983       +2     
  Lines      159159   160693    +1534     
==========================================
+ Hits       117690   118896    +1206     
- Misses      41469    41797     +328     
Flag Coverage Δ
rust 73.98% <82.89%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
src/batch/src/executor/row_seq_scan.rs 20.00% <0.00%> (-0.21%) ⬇️
src/batch/src/lib.rs 100.00% <ø> (ø)
src/frontend/src/handler/variable.rs 0.00% <0.00%> (ø)
...ntend/src/optimizer/plan_node/stream_index_scan.rs 41.40% <0.00%> (-0.33%) ⬇️
src/frontend/src/session.rs 40.36% <0.00%> (ø)
src/source/src/lib.rs 90.00% <ø> (ø)
src/source/src/parser/common.rs 63.63% <ø> (ø)
src/sqlparser/src/keywords.rs 100.00% <ø> (ø)
src/storage/src/hummock/event_handler/mod.rs 0.00% <ø> (ø)
src/storage/src/hummock/local_version/mod.rs 100.00% <ø> (ø)
... and 53 more

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

@StrikeW
Copy link
Contributor

StrikeW commented Nov 18, 2022

Is this PR related to the Unify the materialized source and table RFC?

@xx01cyx
Copy link
Contributor Author

xx01cyx commented Nov 18, 2022

Is this PR related to the Unify the materialized source and table RFC?

Yes. The tracking issue is here.

xx01cyx and others added 2 commits November 18, 2022 11:56
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Contributor

@tabVersion tabVersion left a comment

Choose a reason for hiding this comment

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

generally LGTM

Copy link
Contributor

@st1page st1page left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify
Copy link
Contributor

mergify bot commented Nov 20, 2022

Hey @xx01cyx, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by clicking "Update branch" or pushing an empty commit with git commit --allow-empty -m "rerun" && git push.

@xx01cyx xx01cyx force-pushed the cyx/source-executor-v2 branch from 9a442ef to 7258f7b Compare November 21, 2022 03:16
@mergify
Copy link
Contributor

mergify bot commented Nov 21, 2022

Hey @xx01cyx, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by clicking "Update branch" or pushing an empty commit with git commit --allow-empty -m "rerun" && git push.

@mergify mergify bot merged commit d0a720b into main Nov 21, 2022
@mergify mergify bot deleted the cyx/source-executor-v2 branch November 21, 2022 11:13
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.

4 participants