-
Notifications
You must be signed in to change notification settings - Fork 153
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
[MINOR] feat(server): Introduce LocalFileBlackHoleWriter for test flush ceiling performance #2321
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2321 +/- ##
============================================
+ Coverage 51.19% 51.59% +0.39%
- Complexity 2936 2973 +37
============================================
Files 486 475 -11
Lines 24720 22692 -2028
Branches 2041 2090 +49
============================================
- Hits 12656 11708 -948
+ Misses 11274 10239 -1035
+ Partials 790 745 -45 ☔ View full report in Codecov by Sentry. |
import org.apache.uniffle.storage.common.FileBasedShuffleSegment; | ||
|
||
/** A shuffle writer that write data into black hole. */ | ||
public class LocalFileBlackHoleWriter implements FileWriter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this class should be in the test package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could not specify it for performance test purpose if we put it into the test package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I got it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You would better add more comments to explain how to use this class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
…used for performance test purpose only
@jerqi Add more comments. PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
What changes were proposed in this pull request?
Introduce a new implementation of FileWriter, named
LocalFileBlackHoleWriter
.Why are the changes needed?
Drop all the data, specify
LocalFileBlackHoleWriter
for test the ceiling performance of flush event.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Tested on our pressure cluster.