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

[FEATURE] Support Flink RSS. #1366

Closed
wants to merge 10 commits into from
Closed

Conversation

slfan1989
Copy link
Collaborator

@slfan1989 slfan1989 commented Dec 13, 2023

What changes were proposed in this pull request?

I. Introduction

A. Background

  1. Limitations or issues with the current system
  2. Significance and necessity of the proposed solution

image

  • IntermediateDataSet

An intermediate data set is the data set produced by an operator - either a source or any intermediate operation.

Why are the changes needed?

(Please clarify why the changes are needed. For instance,

  1. If you propose a new API, clarify the use case for a new API.
  2. If you fix a bug, describe the bug.)

Fix: # (issue)

Does this PR introduce any user-facing change?

(Please list the user-facing changes introduced by your change, including

  1. Change in user-facing APIs.
  2. Addition or removal of property keys.)

No.

How was this patch tested?

(Please test your changes, and provide instructions on how to test it:

  1. If you add a feature or fix a bug, add a test to cover your changes.
  2. If you fix a flaky test, repeat it for many times to prove it works.)

@slfan1989 slfan1989 marked this pull request as draft December 13, 2023 03:28
@slfan1989
Copy link
Collaborator Author

@jerqi @zuston I started to submit the code about Flink rss. I still need to spend some time merging the local code. It will take about 2-3 days to complete the submission.

@slfan1989 slfan1989 changed the title flink-rss. [FEATURE] Support Flink RSS. Dec 13, 2023
@codecov-commenter
Copy link

codecov-commenter commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ecbf2e7) 53.22% compared to head (6013879) 54.11%.
Report is 15 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1366      +/-   ##
============================================
+ Coverage     53.22%   54.11%   +0.89%     
- Complexity     2715     2720       +5     
============================================
  Files           418      399      -19     
  Lines         23932    21633    -2299     
  Branches       2043     2051       +8     
============================================
- Hits          12738    11707    -1031     
+ Misses        10408     9208    -1200     
+ Partials        786      718      -68     

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

@slfan1989
Copy link
Collaborator Author

@chenglong-bigdata Can you help review the code?

@slfan1989 slfan1989 self-assigned this Dec 15, 2023
@slfan1989
Copy link
Collaborator Author

public void registerJob(JobShuffleContext context) {
JobID jobID = context.getJobId();
if (shuffleWriteClient == null) {
synchronized (RssShuffleMaster.class) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If it is a class object here, it may be heavier. Is the object RssShuffleMaster itself enough?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you helping to review the code! This PR still needs to be improved.

@chenglong-bigdata
Copy link

@chenglong-bigdata Can you help review the code?

ok

@advancedxy
Copy link
Contributor

Is there a design doc for this one? I can help review this feature.

@slfan1989
Copy link
Collaborator Author

Is there a design doc for this one? I can help review this feature.

Thank you for reviewing the code! The documentation is being improved. I will update it as soon as possible. Unit tests are still being added.

@slfan1989
Copy link
Collaborator Author

I will continue to follow up on this PR.

@slfan1989
Copy link
Collaborator Author

@chenglong-bigdata We will continue to follow up on this PR.

@slfan1989 slfan1989 closed this May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants