[#1938] improvement: Use ShuffleSegment to replace FileBasedShuffleSegment and BufferSegment
What changes were proposed in this pull request?
Now the BufferSegment class and FileBasedShuffleSegment class both six fields, represents a segment.
private long blockId;
private long offset;
private int length;
private int uncompressLength;
private long crc;
private long taskAttemptId;
To add a new ShuffleSegment to replace these two class can make the code easier to maintain.
Why are the changes needed?
To simplify the code.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Exists UT
Test Results
2 717 files +32 2 717 suites +32 5h 35m 51s :stopwatch: + 12m 0s 968 tests ± 0 967 :white_check_mark: + 2 1 :zzz: ±0 0 :x: - 2 12 129 runs +61 12 114 :white_check_mark: +63 15 :zzz: ±0 0 :x: - 2
Results for commit 0687ceeb. ± Comparison against base commit b5cac305.
This pull request removes 9 and adds 9 tests. Note that renamed tests count towards both.
org.apache.uniffle.common.BufferSegmentTest ‑ testEquals
org.apache.uniffle.common.BufferSegmentTest ‑ testGetOffset
org.apache.uniffle.common.BufferSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[1]
org.apache.uniffle.common.BufferSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[2]
org.apache.uniffle.common.BufferSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[3]
org.apache.uniffle.common.BufferSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[4]
org.apache.uniffle.common.BufferSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[5]
org.apache.uniffle.common.BufferSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[6]
org.apache.uniffle.common.BufferSegmentTest ‑ testToString
org.apache.uniffle.common.ShuffleSegmentTest ‑ testEquals
org.apache.uniffle.common.ShuffleSegmentTest ‑ testGetOffset
org.apache.uniffle.common.ShuffleSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[1]
org.apache.uniffle.common.ShuffleSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[2]
org.apache.uniffle.common.ShuffleSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[3]
org.apache.uniffle.common.ShuffleSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[4]
org.apache.uniffle.common.ShuffleSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[5]
org.apache.uniffle.common.ShuffleSegmentTest ‑ testNotEquals{long, long, int, int, long, long}[6]
org.apache.uniffle.common.ShuffleSegmentTest ‑ testToString
:recycle: This comment has been updated with latest results.
Sorry for the late reply. @wankunde
Thanks for proposing this. Actually the ShuffleSegment has been introduce to be as the an abstract class. And I think the initial design is to clarify the client/server side shuffleSegment. But I agree with the same part 6 variables should be defined in the existing ShuffleSegment. And there is no need to make the invoking side to use the ShuffleSegment to replace all the Filebased or Buffer segnment.