You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "onebox-li (via GitHub)" <gi...@apache.org> on 2023/07/18 10:53:32 UTC
[GitHub] [incubator-celeborn] onebox-li opened a new pull request, #1730: [CELEBORN-807] Adjust shutdown worker logs in LifecycleManager
onebox-li opened a new pull request, #1730:
URL: https://github.com/apache/incubator-celeborn/pull/1730
### What changes were proposed in this pull request?
In a long run celeborn cluster, there are some shutdown workers. Whether it is a new task or an old task, even if the worker is not assigned , it will always log below, seems a little noisy.
ERROR CommitManager: Worker xx shutdown, commit all it's partition location.
### Why are the changes needed?
Ditto
### Does this PR introduce _any_ user-facing change?
shutdown worker logs in LifecycleManager changes
### How was this patch tested?
manually test
--
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 #1730: [CELEBORN-807] Adjust shutdown worker logs in LifecycleManager
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #1730:
URL: https://github.com/apache/incubator-celeborn/pull/1730#issuecomment-1640011926
## [Codecov](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1730?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#1730](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1730?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (2972d3c) into [main](https://app.codecov.io/gh/apache/incubator-celeborn/commit/16d9c657c2b0605d8708a62392bb10b98749b2aa?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (16d9c65) will **not change** coverage.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## main #1730 +/- ##
=======================================
Coverage 46.60% 46.60%
=======================================
Files 161 161
Lines 10029 10029
Branches 927 927
=======================================
Hits 4673 4673
Misses 5036 5036
Partials 320 320
```
[see 2 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1730/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
: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=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
[GitHub] [incubator-celeborn] pan3793 commented on a diff in pull request #1730: [CELEBORN-807] Adjust shutdown worker logs in LifecycleManager
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1730:
URL: https://github.com/apache/incubator-celeborn/pull/1730#discussion_r1266940412
##########
client/src/main/scala/org/apache/celeborn/client/WorkerStatusTracker.scala:
##########
@@ -198,10 +207,11 @@ class WorkerStatusTracker(
}
}
}
-
- logInfo(
- s"Current excluded workers $excludedWorkers, Current shuttingDown ${shuttingWorkers.asScala.map(
- _.readableAddress()).mkString("\n")}")
+ if (statusChanged) {
+ logInfo(
+ s"Current excluded workers $excludedWorkers, " +
+ s"Current shutting down workers ${shuttingWorkers.asScala.map(_.readableAddress()).mkString("\n")}")
Review Comment:
how about the following? (I write it by hand you may need to call `dev/reformat`)
```suggestion
logInfo(
s"""
|Current excluded workers:
|$excludedWorkers.mkString("\n")
|
|Current shutting down workers:
|${shuttingWorkers.asScala.map(_.readableAddress()).mkString("\n")}"
|"""stripMargin)
```
--
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] onebox-li commented on a diff in pull request #1730: [CELEBORN-807] Adjust shutdown worker logs in LifecycleManager
Posted by "onebox-li (via GitHub)" <gi...@apache.org>.
onebox-li commented on code in PR #1730:
URL: https://github.com/apache/incubator-celeborn/pull/1730#discussion_r1267499517
##########
client/src/main/scala/org/apache/celeborn/client/CommitManager.scala:
##########
@@ -250,6 +250,10 @@ class CommitManager(appUniqueId: String, val conf: CelebornConf, lifecycleManage
getCommitHandler(shuffleId).isStageEnd(shuffleId)
}
+ def isStageEndOrInProcess(shuffleId: Int): Boolean = {
Review Comment:
i just follow `isStageEnd` usage in `CommitManager`
--
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] pan3793 commented on pull request #1730: [CELEBORN-807] Adjust shutdown worker logs in LifecycleManager
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #1730:
URL: https://github.com/apache/incubator-celeborn/pull/1730#issuecomment-1641317655
@onebox-li thanks, I agree that we should suppress the noisy logs. I have experienced that the client excluded all workers and then stuck, will this change help or make it hard to identify such cases?
--
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] onebox-li commented on pull request #1730: [CELEBORN-807] Adjust shutdown worker logs in LifecycleManager
Posted by "onebox-li (via GitHub)" <gi...@apache.org>.
onebox-li commented on PR #1730:
URL: https://github.com/apache/incubator-celeborn/pull/1730#issuecomment-1641359199
> @onebox-li thanks, I agree that we should suppress the noisy logs. I have experienced that the client excluded all workers and then stuck, will this change help or make it hard to identify such cases?
I think it may help a bit, too many same logs seem harder to troubleshoot and drown out other important logs.
--
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 #1730: [CELEBORN-807] Adjust shutdown worker logs in LifecycleManager
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture commented on code in PR #1730:
URL: https://github.com/apache/incubator-celeborn/pull/1730#discussion_r1267492823
##########
client/src/main/scala/org/apache/celeborn/client/CommitManager.scala:
##########
@@ -250,6 +250,10 @@ class CommitManager(appUniqueId: String, val conf: CelebornConf, lifecycleManage
getCommitHandler(shuffleId).isStageEnd(shuffleId)
}
+ def isStageEndOrInProcess(shuffleId: Int): Boolean = {
Review Comment:
nit: IMO it's unnecessary to add this method.
--
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] onebox-li commented on a diff in pull request #1730: [CELEBORN-807] Adjust shutdown worker logs in LifecycleManager
Posted by "onebox-li (via GitHub)" <gi...@apache.org>.
onebox-li commented on code in PR #1730:
URL: https://github.com/apache/incubator-celeborn/pull/1730#discussion_r1267466159
##########
client/src/main/scala/org/apache/celeborn/client/WorkerStatusTracker.scala:
##########
@@ -198,10 +207,11 @@ class WorkerStatusTracker(
}
}
}
-
- logInfo(
- s"Current excluded workers $excludedWorkers, Current shuttingDown ${shuttingWorkers.asScala.map(
- _.readableAddress()).mkString("\n")}")
+ if (statusChanged) {
+ logInfo(
+ s"Current excluded workers $excludedWorkers, " +
+ s"Current shutting down workers ${shuttingWorkers.asScala.map(_.readableAddress()).mkString("\n")}")
Review Comment:
OK, done
--
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 closed pull request #1730: [CELEBORN-807] Adjust shutdown worker logs in LifecycleManager
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture closed pull request #1730: [CELEBORN-807] Adjust shutdown worker logs in LifecycleManager
URL: https://github.com/apache/incubator-celeborn/pull/1730
--
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