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(source): support source failover & refactor #2257

Merged
merged 10 commits into from
May 6, 2022
Merged

Conversation

tabVersion
Copy link
Contributor

@tabVersion tabVersion commented Apr 30, 2022

Signed-off-by: tabVersion [email protected]

What's changed and what's your intention?

PLEASE DO NOT LEAVE THIS EMPTY !!!

Please explain IN DETAIL what the changes are in this PR and why they are needed:

support failover.

  • introduce DummyConnector, it will serve as an idle connector for the executor which is assigned no split.

rename SourceSplit -> SplitMetaData:

- fn to_string(&self) -> Result<String>
+ fn to_json_bytes(&self) -> Result<Bytes>
  • Summarize your change (mandatory)
  • How does this PR work? Need a brief introduction for the changed logic (optional)
  • Describe clearly one logical change and avoid lazy messages (optional)
  • Describe any limitations of the current code (optional)

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)

#1963

Signed-off-by: tabVersion <[email protected]>
Signed-off-by: tabVersion <[email protected]>
@codecov
Copy link

codecov bot commented Apr 30, 2022

Codecov Report

Merging #2257 (2460bc8) into main (0193870) will decrease coverage by 0.09%.
The diff coverage is 23.52%.

@@            Coverage Diff             @@
##             main    #2257      +/-   ##
==========================================
- Coverage   71.07%   70.98%   -0.10%     
==========================================
  Files         675      676       +1     
  Lines       84502    84736     +234     
==========================================
+ Hits        60061    60150      +89     
- Misses      24441    24586     +145     
Flag Coverage Δ
rust 70.98% <23.52%> (-0.10%) ⬇️

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

Impacted Files Coverage Δ
src/connector/src/base.rs 2.89% <0.00%> (-2.11%) ⬇️
src/connector/src/dummy_connector.rs 0.00% <0.00%> (ø)
...nnector/src/filesystem/s3/source/s3_file_reader.rs 0.00% <0.00%> (ø)
src/connector/src/kafka/split.rs 0.00% <0.00%> (ø)
src/connector/src/kinesis/split.rs 0.00% <0.00%> (ø)
src/connector/src/lib.rs 100.00% <ø> (ø)
src/connector/src/nexmark/enumerator/mod.rs 100.00% <ø> (ø)
src/connector/src/nexmark/split.rs 23.52% <0.00%> (ø)
src/connector/src/pulsar/split.rs 0.00% <0.00%> (ø)
src/meta/src/stream/stream_manager.rs 64.19% <0.00%> (ø)
... and 20 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

src/connector/src/state.rs Outdated Show resolved Hide resolved
pub struct ConnectorState {
pub identifier: Bytes,
pub start_offset: String,
pub end_offset: String,
}

impl SplitMetaData for ConnectorState {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think ConnectorState is not a SplitMetaData? It's not going to be used by list_splits().

Suggested change
impl SplitMetaData for ConnectorState {
impl ConnectorState {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The contents of the SourceStateHandler store need to implement the trait.

* stage

Signed-off-by: tabVersion <[email protected]>

* load state before stream begins

Signed-off-by: tabVersion <[email protected]>

* bring stream chunk and offset to executor

Signed-off-by: tabVersion <[email protected]>

* modify stream format to either

Signed-off-by: tabVersion <[email protected]>

* use future::pending instead of dead loop

Signed-off-by: tabVersion <[email protected]>

* make split_offset_mapping an option

Signed-off-by: tabVersion <[email protected]>
@tabVersion tabVersion changed the title refactor(source): rename SourceSplit -> SplitMetaData feat(source): support source failover & refactor May 6, 2022
src/connector/src/base.rs Outdated Show resolved Hide resolved
src/connector/src/base.rs Show resolved Hide resolved
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 784 files.

Valid Invalid Ignored Fixed
782 1 1 0
Click to see the invalid file list
  • src/connector/src/dummy_connector.rs

src/connector/src/dummy_connector.rs Show resolved Hide resolved
Signed-off-by: tabVersion <[email protected]>
Signed-off-by: tabVersion <[email protected]>
Copy link
Contributor

@neverchanje neverchanje left a comment

Choose a reason for hiding this comment

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

lgtm, StreamChunkWithState looks weird to me but anyway we can probably try another approach later.

@tabVersion tabVersion merged commit 1af0bae into main May 6, 2022
@tabVersion tabVersion deleted the tab/source-refactor branch May 6, 2022 09:26
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.

2 participants