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

[#416] feat(hdfs): lazy initialization of hdfsShuffleWriteHandler when per-partition concurrent write is enabled #816

Merged
merged 4 commits into from
Apr 17, 2023

Conversation

zuston
Copy link
Member

@zuston zuston commented Apr 12, 2023

What changes were proposed in this pull request?

lazy initialization of hdfsShuffleWriteHandler when per-partition concurrent write is enabled

Why are the changes needed?

Without this PR, it will create too much unnecessary, when enable the
concurrent write of per-partition and no-race condition.

This PR is to reduce unnecessary handler creation.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

  1. UTs

@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2023

Codecov Report

Merging #816 (663349a) into master (cae7cd9) will increase coverage by 1.40%.
The diff coverage is 72.22%.

@@             Coverage Diff              @@
##             master     #816      +/-   ##
============================================
+ Coverage     57.63%   59.03%   +1.40%     
- Complexity     2058     2061       +3     
============================================
  Files           306      292      -14     
  Lines         14871    12912    -1959     
  Branches       1221     1222       +1     
============================================
- Hits           8571     7623     -948     
+ Misses         5808     4857     -951     
+ Partials        492      432      -60     
Impacted Files Coverage Δ
...ge/handler/impl/PooledHdfsShuffleWriteHandler.java 69.44% <72.22%> (+25.96%) ⬆️

... and 14 files with indirect coverage changes

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

Copy link
Contributor

@jerqi jerqi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @zuston

@jerqi
Copy link
Contributor

jerqi commented Apr 17, 2023

Could you raise a #773 backport pr to branch-0.7? #773 (comment)

@zuston zuston merged commit 23e0a51 into apache:master Apr 17, 2023
@zuston
Copy link
Member Author

zuston commented Apr 17, 2023

Could you raise a #773 backport pr to branch-0.7? #773 (comment)

Done.

@jerqi
Copy link
Contributor

jerqi commented Apr 17, 2023

Could you raise a #773 backport pr to branch-0.7? #773 (comment)

Done.

CI failed. You can't cherry-pick this pr. You should revert it and raise a new pr. There is conflict with branch 0.7.

@jerqi
Copy link
Contributor

jerqi commented Apr 17, 2023

I have reverted your mistake commit. You should raise a new pr.

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.

3 participants