You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "AngersZhuuuu (via GitHub)" <gi...@apache.org> on 2024/01/12 03:32:12 UTC
[PR] [CELEBORN-1220][IMPROVEMENT] Make trim logic more robust [incubator-celeborn]
AngersZhuuuu opened a new pull request, #2224:
URL: https://github.com/apache/incubator-celeborn/pull/2224
### What changes were proposed in this pull request?
We meet a case that trim action stoped but didn't set trimInProcess back, then the worker won't trigger a new trim and pause push data, already pushed data(replicate data can work well) won't release, then won't recover receive push data request.
This pr make the logic more robust
### Why are the changes needed?
Make logic more robust
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
No
--
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
Re: [PR] [CELEBORN-1220][IMPROVEMENT] Make trim logic more robust [incubator-celeborn]
Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX commented on PR #2224:
URL: https://github.com/apache/incubator-celeborn/pull/2224#issuecomment-1891375550
Merged into main(v0.5.0) and branch-0.4(v0.4.0)
--
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
Re: [PR] [CELEBORN-1220][IMPROVEMENT] Make trim logic more robust [incubator-celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #2224:
URL: https://github.com/apache/incubator-celeborn/pull/2224#discussion_r1449942666
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/memory/MemoryManager.java:
##########
@@ -317,10 +317,15 @@ public void trimAllListeners() {
if (trimInProcess.compareAndSet(false, true)) {
actionService.submit(
() -> {
- // In current code, StorageManager will add into this before ChannelsLimiter,
- // so all behaviors of StorageManger will execute before ChannelsLimiter.
- memoryPressureListeners.forEach(MemoryPressureListener::onTrim);
- trimInProcess.set(false);
+ try {
+ // In current code, StorageManager will add into this before ChannelsLimiter,
+ // so all behaviors of StorageManger will execute before ChannelsLimiter.
+ memoryPressureListeners.forEach(MemoryPressureListener::onTrim);
+ } finally {
+ // MemoryManager use this flag to avoid trigger trim action parallel,
Review Comment:
```suggestion
// MemoryManager uses this flag to avoid parallel trigger trim action,
```
--
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
Re: [PR] [CELEBORN-1220][IMPROVEMENT] Make trim logic more robust [incubator-celeborn]
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on PR #2224:
URL: https://github.com/apache/incubator-celeborn/pull/2224#issuecomment-1888765536
better merge to 0.3.2 and 0.4.0 cc @cfmcgrady @SteNicholas
--
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
Re: [PR] [CELEBORN-1220][IMPROVEMENT] Make trim logic more robust [incubator-celeborn]
Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX closed pull request #2224: [CELEBORN-1220][IMPROVEMENT] Make trim logic more robust
URL: https://github.com/apache/incubator-celeborn/pull/2224
--
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
Re: [PR] [CELEBORN-1220][IMPROVEMENT] Make trim logic more robust [incubator-celeborn]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #2224:
URL: https://github.com/apache/incubator-celeborn/pull/2224#issuecomment-1888388291
## [Codecov](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2224?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`7bc3497`)](https://app.codecov.io/gh/apache/incubator-celeborn/commit/7bc34972ac9d5c4bc6f7d96e857f7cbe5ec23986?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 47.22% compared to head [(`266bab1`)](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2224?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 47.22%.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #2224 +/- ##
=======================================
Coverage 47.22% 47.22%
=======================================
Files 192 192
Lines 11778 11778
Branches 1050 1050
=======================================
Hits 5561 5561
Misses 5844 5844
Partials 373 373
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2224?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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
Re: [PR] [CELEBORN-1220][IMPROVEMENT] Make trim logic more robust [incubator-celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #2224:
URL: https://github.com/apache/incubator-celeborn/pull/2224#discussion_r1449943071
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/memory/MemoryManager.java:
##########
@@ -317,10 +317,15 @@ public void trimAllListeners() {
if (trimInProcess.compareAndSet(false, true)) {
actionService.submit(
() -> {
- // In current code, StorageManager will add into this before ChannelsLimiter,
- // so all behaviors of StorageManger will execute before ChannelsLimiter.
- memoryPressureListeners.forEach(MemoryPressureListener::onTrim);
- trimInProcess.set(false);
+ try {
+ // In current code, StorageManager will add into this before ChannelsLimiter,
+ // so all behaviors of StorageManger will execute before ChannelsLimiter.
+ memoryPressureListeners.forEach(MemoryPressureListener::onTrim);
+ } finally {
+ // MemoryManager use this flag to avoid trigger trim action parallel,
+ // We should make sure set this value back, or it won't trigger trim action again.
Review Comment:
```suggestion
// We should make sure set this value back, otherwise it won't trigger trim action again.
```
--
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