You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "wankunde (via GitHub)" <gi...@apache.org> on 2024/07/21 15:23:14 UTC
[PR] [WIP][#1938] improvement: Use ShuffleSegment to replace FileBasedShuffleSegment and BufferSegment [incubator-uniffle]
wankunde opened a new pull request, #1939:
URL: https://github.com/apache/incubator-uniffle/pull/1939
### What changes were proposed in this pull request?
Now the BufferSegment class and FileBasedShuffleSegment class both six fields, represents a segment.
```java
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1938] improvement: Use ShuffleSegment to replace FileBasedShuffleSegment and BufferSegment [incubator-uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #1939:
URL: https://github.com/apache/incubator-uniffle/pull/1939#issuecomment-2295568254
Sorry for the late reply.
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [WIP][#1938] improvement: Use ShuffleSegment to replace FileBasedShuffleSegment and BufferSegment [incubator-uniffle]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1939:
URL: https://github.com/apache/incubator-uniffle/pull/1939#issuecomment-2241693687
## Test Results
2 717 files +32 2 717 suites +32 5h 36m 10s :stopwatch: + 12m 19s
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 1425a7a5. ± Comparison against base commit b5cac305.
<details>
<summary>This pull request <b>removes</b> 9 and <b>adds</b> 9 tests. <i>Note that renamed tests count towards both.</i></summary>
```
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
```
</details>
[test-results]:data:application/gzip;base64,H4sIAIMtnWYC/03MTQ6DIBCG4asY1l04yDDQyxiKkpD60yCsTO9eNEJdfs9M3p05P40bezacgB4N25KP9z2kYKJfl0NaoDZTvsfjQ0tVVr8la0+iP739JxNUcMZPGWqiH0NYwyUhLUcTOHB9zRLNBqLaVcUCt+q571G7zrOPeTAQHA0ZBCVlNyhEx63jIDShIKdVZ2TbAb7Y9wf14Ie2EgEAAA==
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org