You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by GitBox <gi...@apache.org> on 2023/01/16 09:26:30 UTC
[GitHub] [incubator-celeborn] FMX opened a new pull request, #1168: [CELEBORN-228]Refactor PartitionFileSorter to avoid specific JDK dependency.
FMX opened a new pull request, #1168:
URL: https://github.com/apache/incubator-celeborn/pull/1168
### What changes were proposed in this pull request?
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
### How was this patch tested?
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] AngersZhuuuu commented on a diff in pull request #1168: [CELEBORN-228]Refactor PartitionFileSorter to avoid specific JDK dependency.
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on code in PR #1168:
URL: https://github.com/apache/incubator-celeborn/pull/1168#discussion_r1132139965
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/storage/PartitionFilesSorter.java:
##########
@@ -656,14 +646,41 @@ private void deleteOriginFiles() throws IOException {
}
}
- private ByteBuffer expandBufferAndUpdateMemoryTracker(int oldCapacity, int newCapacity)
- throws InterruptedException {
- memoryManager.releaseSortMemory(oldCapacity);
- memoryManager.reserveSortMemory(newCapacity);
Review Comment:
if newCapacity > maxSortMemoryRatio, will cause `sortMemoryReady()` return false and this memory can't release, then cause this thread stuck here and cause PartitionSorter can't take FileSorter to executor.
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] codecov[bot] commented on pull request #1168: [CELEBORN-228]Refactor PartitionFileSorter to avoid specific JDK dependency.
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #1168:
URL: https://github.com/apache/incubator-celeborn/pull/1168#issuecomment-1383762493
# [Codecov](https://codecov.io/gh/apache/incubator-celeborn/pull/1168?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1168](https://codecov.io/gh/apache/incubator-celeborn/pull/1168?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (50f26bf) into [main](https://codecov.io/gh/apache/incubator-celeborn/commit/810a8d01e083d8df663bcd9482f0d1178fbe1ccc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (810a8d0) will **increase** coverage by `0.21%`.
> The diff coverage is `36.12%`.
```diff
@@ Coverage Diff @@
## main #1168 +/- ##
============================================
+ Coverage 26.88% 27.08% +0.21%
- Complexity 747 766 +19
============================================
Files 200 200
Lines 17022 17194 +172
Branches 1838 1867 +29
============================================
+ Hits 4574 4655 +81
- Misses 12151 12222 +71
- Partials 297 317 +20
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-celeborn/pull/1168?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ce/deploy/worker/storage/PartitionFilesSorter.java](https://codecov.io/gh/apache/incubator-celeborn/pull/1168?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d29ya2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jZWxlYm9ybi9zZXJ2aWNlL2RlcGxveS93b3JrZXIvc3RvcmFnZS9QYXJ0aXRpb25GaWxlc1NvcnRlci5qYXZh) | `65.35% <34.29%> (-1.79%)` | :arrow_down: |
| [...cala/org/apache/celeborn/common/CelebornConf.scala](https://codecov.io/gh/apache/incubator-celeborn/pull/1168?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvY2VsZWJvcm4vY29tbW9uL0NlbGVib3JuQ29uZi5zY2FsYQ==) | `80.62% <100.00%> (-0.07%)` | :arrow_down: |
| [...service/deploy/worker/storage/StorageManager.scala](https://codecov.io/gh/apache/incubator-celeborn/pull/1168?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d29ya2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvY2VsZWJvcm4vc2VydmljZS9kZXBsb3kvd29ya2VyL3N0b3JhZ2UvU3RvcmFnZU1hbmFnZXIuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
| [.../org/apache/celeborn/client/ShuffleClientImpl.java](https://codecov.io/gh/apache/incubator-celeborn/pull/1168?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jZWxlYm9ybi9jbGllbnQvU2h1ZmZsZUNsaWVudEltcGwuamF2YQ==) | `19.72% <0.00%> (+0.32%)` | :arrow_up: |
| [...ice/deploy/master/clustermeta/ha/HARaftServer.java](https://codecov.io/gh/apache/incubator-celeborn/pull/1168?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jZWxlYm9ybi9zZXJ2aWNlL2RlcGxveS9tYXN0ZXIvY2x1c3Rlcm1ldGEvaGEvSEFSYWZ0U2VydmVyLmphdmE=) | `77.93% <0.00%> (+1.36%)` | :arrow_up: |
| [.../deploy/worker/storage/MapPartitionFileWriter.java](https://codecov.io/gh/apache/incubator-celeborn/pull/1168?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d29ya2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jZWxlYm9ybi9zZXJ2aWNlL2RlcGxveS93b3JrZXIvc3RvcmFnZS9NYXBQYXJ0aXRpb25GaWxlV3JpdGVyLmphdmE=) | `41.18% <0.00%> (+41.18%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] waitinfuture merged pull request #1168: [CELEBORN-228]Refactor PartitionFileSorter to avoid specific JDK dependency.
Posted by GitBox <gi...@apache.org>.
waitinfuture merged PR #1168:
URL: https://github.com/apache/incubator-celeborn/pull/1168
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-celeborn] waitinfuture commented on a diff in pull request #1168: [CELEBORN-228]Refactor PartitionFileSorter to avoid specific JDK dependency.
Posted by GitBox <gi...@apache.org>.
waitinfuture commented on code in PR #1168:
URL: https://github.com/apache/incubator-celeborn/pull/1168#discussion_r1071137701
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/storage/PartitionFilesSorter.java:
##########
@@ -656,14 +646,68 @@ private void deleteOriginFiles() throws IOException {
}
}
- private ByteBuffer expandBufferAndUpdateMemoryTracker(int oldCapacity, int newCapacity)
- throws InterruptedException {
- memoryManager.releaseSortMemory(oldCapacity);
- memoryManager.reserveSortMemory(newCapacity);
- while (!memoryManager.sortMemoryReady()) {
- Thread.sleep(20);
+ protected void readStreamBySize(
Review Comment:
I think we don't need this for HDFS
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org