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.

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

bug: empty source manager in newly joined compute nodes #5269

Closed
BugenZhao opened this issue Sep 12, 2022 · 1 comment
Closed

bug: empty source manager in newly joined compute nodes #5269

BugenZhao opened this issue Sep 12, 2022 · 1 comment
Assignees
@yezizp2012 @PanQL
Labels
component/streaming Stream processing related issue. help wanted Issues that need help from contributors type/bug Something isn't working type/enhancement Improvements to existing implementation.
Milestone
release-0.1.14

Comments

@BugenZhao
Copy link
Member

If we reschedule source actors to newly joined CN, there will be anther thing need to be considering: the local source manager in new CN does not contains related source desc.

Exactly. Do we have sync mechanism for sources now? 😢

Nope, only RPC calls. 🥵 Or we can let CN subscribe source info, it's a little bit costly but works.

I'm not sure which pattern will be better for this, subscribing the source info, or totally removing the local source manager. As it's only required for table sources now, we may find a workaround way for this. 🤔

Totally removing the local source manager seems not that easy currently, especially for table sources.
IMO, if using subscribe mode, since create_source RPC in CN side is still needed for creating source ddl request (replacing with notification is not reliable as we discussed before), this will bring different behaviors between scheduling source actor and creating source. And CN should only subscribe source snapshot and not source info changes, which is quit weird.
If using RPC call here, we'd better make create_source able to accept duplicate sources. Because we might reschedule actors in same source multi times.

Originally posted by @yezizp2012 in #5226 (comment)
@github-actions github-actions bot added this to the release-0.1.13 milestone Sep 12, 2022
@BugenZhao BugenZhao added type/bug Something isn't working component/streaming Stream processing related issue. type/enhancement Improvements to existing implementation. help wanted Issues that need help from contributors labels Sep 12, 2022
@BugenZhao BugenZhao mentioned this issue Sep 12, 2022
Merged
3 tasks
@PanQL PanQL mentioned this issue Sep 30, 2022
Merged
3 tasks
mergify bot pushed a commit that referenced this issue Oct 6, 2022
@PanQL @BugenZhao
refactor: remove create_source/sync_source rpc in CN(#5269) (#5654)
bd3bd59 
* refactor: wrap source creation into SourceDescBuilder to create stream source.

Signed-off-by: Qinglin Pan <[email protected]>

* remove StreamSource related part of create_source rpc in CN

Signed-off-by: Qinglin Pan <[email protected]>

* fix typo

Signed-off-by: Qinglin Pan <[email protected]>

* fix unit test

Signed-off-by: Qinglin Pan <[email protected]>

* remove SourceType field in SourceNode

Signed-off-by: Qinglin Pan <[email protected]>

* remap row_id_index in SourceExecutor

Signed-off-by: Qinglin Pan <[email protected]>

* build table source in SourceDescBuilder, remove create_source/sync_source rpc

- move create_table_source logic into SourceDescBuilder
- remove create_source rpc in CN
- remove sync_source rpc in CN
- fix all unittests' complaination

Signed-off-by: Qinglin Pan <[email protected]>

* fix misc check error

Signed-off-by: Qinglin Pan <[email protected]>

* move create_table_info into source::table::test_utils

Signed-off-by: Qinglin Pan <[email protected]>

* remove row_id_index field from source_executor

Signed-off-by: Qinglin Pan <[email protected]>

* remove SourceCatalogType in frontend

Signed-off-by: Qinglin Pan <[email protected]>

* Update src/stream/src/executor/source/source_executor.rs

Co-authored-by: Bugen Zhao <[email protected]>
Signed-off-by: Qinglin Pan <[email protected]>

Signed-off-by: Qinglin Pan <[email protected]>
Co-authored-by: Bugen Zhao <[email protected]>
@BugenZhao
Copy link
Member Author

This should be fixed by #5654 and #5849.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yezizp2012 yezizp2012

@PanQL PanQL

Labels
component/streaming Stream processing related issue. help wanted Issues that need help from contributors type/bug Something isn't working type/enhancement Improvements to existing implementation.
Projects
None yet
Milestone
release-0.1.14
Development

No branches or pull requests
4 participants
@fuyufjh @yezizp2012 @BugenZhao @PanQL