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

[issue-717] Let Pravega internal thread pool deal with checkpoint #718

Merged
merged 4 commits into from
Oct 11, 2023

Conversation

crazyzhou
Copy link
Contributor

Change log description
Use ReaderGroup::initiateCheckpoint(checkpointName) to perform the checkpoint

Purpose of the change
Fixes #717

What the code does
Update the method
Update the Pravega version

How to verify it
./gradlew clean build passes

Signed-off-by: Brian Zhou <b.zhou@dell.com>
Signed-off-by: Brian Zhou <b.zhou@dell.com>
Copy link

@ShwethaSNayak ShwethaSNayak left a comment

Choose a reason for hiding this comment

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

LGTM, Looks like build is failed.

@crazyzhou
Copy link
Contributor Author

The test failure is because of this code change, looks like the change introduce a deadlock in the client internal thread in the read group. We cannot make such change.

Signed-off-by: Brian Zhou <b.zhou@dell.com>
Signed-off-by: Brian Zhou <b.zhou@dell.com>
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (1157b7b) 81.12% compared to head (c5c1ea4) 81.23%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #718      +/-   ##
============================================
+ Coverage     81.12%   81.23%   +0.11%     
- Complexity      573      574       +1     
============================================
  Files            62       62              
  Lines          2723     2723              
  Branches        232      232              
============================================
+ Hits           2209     2212       +3     
+ Misses          326      324       -2     
+ Partials        188      187       -1     
Files Coverage Δ
...pravega/connectors/flink/ReaderCheckpointHook.java 92.18% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

@crazyzhou crazyzhou merged commit a55bfe1 into pravega:master Oct 11, 2023
7 checks passed
@crazyzhou crazyzhou deleted the issue-717 branch October 11, 2023 10:27
Welkin-Y pushed a commit to Welkin-Y/Pravega-flink-connectors that referenced this pull request Oct 30, 2023
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.

Let Pravega internal thread pool to deal with checkpoint
3 participants