You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "lyy-pineapple (via GitHub)" <gi...@apache.org> on 2024/02/23 07:15:45 UTC
[PR] [CELEBRON-1282]Optimize push data replica error message [incubator-celeborn]
lyy-pineapple opened a new pull request, #2323:
URL: https://github.com/apache/incubator-celeborn/pull/2323
### What changes were proposed in this pull request?
Optimize the handling of exceptions during the push of replica data, now only throwing PUSH_DATA_CONNECTION_EXCEPTION_REPLICA in specific scenarios.
### Why are the changes needed?
When handling exceptions related to pushing replica data in the worker, unmatched exceptions, such as 'file already closed,' are uniformly transformed into REPLICATE_DATA_CONNECTION_EXCEPTION_COUNT and returned to the client. The client then excludes the peer node based on this count, which may not be appropriate in certain scenarios. For instance, in the case of an exception like 'file already closed,' it typically occurs during multiple splits and commitFile operations. Excluding a large number of nodes under such circumstances is clearly not in line with expectations.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
through exist uts
--
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] [CELEBRON-1282]Optimize push data replica error message [incubator-celeborn]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #2323:
URL: https://github.com/apache/incubator-celeborn/pull/2323#issuecomment-1960849258
## [Codecov](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2323?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `37.50000%` with `5 lines` in your changes are missing coverage. Please review.
> Project coverage is 48.85%. Comparing base [(`b9bdea3`)](https://app.codecov.io/gh/apache/incubator-celeborn/commit/b9bdea3c72fcc7ffc45bd24d53e920cea5516c5c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`7e93e92`)](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2323?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
| [Files](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2323?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...rg/apache/celeborn/common/util/ExceptionUtils.java](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2323?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jZWxlYm9ybi9jb21tb24vdXRpbC9FeGNlcHRpb25VdGlscy5qYXZh) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2323?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../scala/org/apache/celeborn/common/util/Utils.scala](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2323?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvY2VsZWJvcm4vY29tbW9uL3V0aWwvVXRpbHMuc2NhbGE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2323?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #2323 +/- ##
==========================================
- Coverage 48.86% 48.85% -0.00%
==========================================
Files 207 207
Lines 12978 12983 +5
Branches 1117 1119 +2
==========================================
+ Hits 6340 6342 +2
- Misses 6233 6236 +3
Partials 405 405
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2323?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] [CELEBRON-1282]Optimize push data replica error message [incubator-celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #2323:
URL: https://github.com/apache/incubator-celeborn/pull/2323#discussion_r1500280939
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/WorkerSource.scala:
##########
@@ -144,6 +145,7 @@ object WorkerSource {
val REPLICATE_DATA_WRITE_FAIL_COUNT = "ReplicateDataWriteFailCount"
val REPLICATE_DATA_CREATE_CONNECTION_FAIL_COUNT = "ReplicateDataCreateConnectionFailCount"
val REPLICATE_DATA_CONNECTION_EXCEPTION_COUNT = "ReplicateDataConnectionExceptionCount"
+ val REPLICATE_DATA_FAIL_NON_CRITICAL_CAUSE_COUNT = "ReplicateDataFailNonCriticalCauseCount"
Review Comment:
Update the dashboard json for `ReplicateDataFailNonCriticalCauseCount"`. Or I will push another followup pr after this pr is merged.
--
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] [CELEBRON-1282]Optimize push data replica error message [incubator-celeborn]
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture commented on code in PR #2323:
URL: https://github.com/apache/incubator-celeborn/pull/2323#discussion_r1501615657
##########
client/src/main/java/org/apache/celeborn/client/ShuffleClientImpl.java:
##########
@@ -1752,7 +1752,7 @@ private StatusCode getPushDataFailCause(String message) {
StatusCode cause;
if (message == null) {
Review Comment:
Shall we also check `PUSH_DATA_FAIL_NON_CRITICAL_CAUSE_REPLICA` here?
--
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] [CELEBRON-1282]Optimize push data replica error message [incubator-celeborn]
Posted by "lyy-pineapple (via GitHub)" <gi...@apache.org>.
lyy-pineapple commented on code in PR #2323:
URL: https://github.com/apache/incubator-celeborn/pull/2323#discussion_r1500504683
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/WorkerSource.scala:
##########
@@ -144,6 +145,7 @@ object WorkerSource {
val REPLICATE_DATA_WRITE_FAIL_COUNT = "ReplicateDataWriteFailCount"
val REPLICATE_DATA_CREATE_CONNECTION_FAIL_COUNT = "ReplicateDataCreateConnectionFailCount"
val REPLICATE_DATA_CONNECTION_EXCEPTION_COUNT = "ReplicateDataConnectionExceptionCount"
+ val REPLICATE_DATA_FAIL_NON_CRITICAL_CAUSE_COUNT = "ReplicateDataFailNonCriticalCauseCount"
Review Comment:
Thank you for creating an additional PR to update the JSON. I have limited understanding in this area.
--
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] [CELEBRON-1282] Optimize push data replica error message [incubator-celeborn]
Posted by "lyy-pineapple (via GitHub)" <gi...@apache.org>.
lyy-pineapple commented on code in PR #2323:
URL: https://github.com/apache/incubator-celeborn/pull/2323#discussion_r1501964534
##########
client/src/main/java/org/apache/celeborn/client/ShuffleClientImpl.java:
##########
@@ -1752,7 +1752,7 @@ private StatusCode getPushDataFailCause(String message) {
StatusCode cause;
if (message == null) {
Review Comment:
Thanks~, fixed
--
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] [CELEBRON-1282] Optimize push data replica error message [incubator-celeborn]
Posted by "waitinfuture (via GitHub)" <gi...@apache.org>.
waitinfuture closed pull request #2323: [CELEBRON-1282] Optimize push data replica error message
URL: https://github.com/apache/incubator-celeborn/pull/2323
--
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] [CELEBRON-1282]Optimize push data replica error message [incubator-celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #2323:
URL: https://github.com/apache/incubator-celeborn/pull/2323#discussion_r1500280939
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/WorkerSource.scala:
##########
@@ -144,6 +145,7 @@ object WorkerSource {
val REPLICATE_DATA_WRITE_FAIL_COUNT = "ReplicateDataWriteFailCount"
val REPLICATE_DATA_CREATE_CONNECTION_FAIL_COUNT = "ReplicateDataCreateConnectionFailCount"
val REPLICATE_DATA_CONNECTION_EXCEPTION_COUNT = "ReplicateDataConnectionExceptionCount"
+ val REPLICATE_DATA_FAIL_NON_CRITICAL_CAUSE_COUNT = "ReplicateDataFailNonCriticalCauseCount"
Review Comment:
Update the dashboard json for `ReplicateDataFailNonCriticalCauseCount"`.
--
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