You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "cxzl25 (via GitHub)" <gi...@apache.org> on 2024/02/26 03:25:40 UTC
[PR] [CELEBORN-1288] Prompt configuration items when receiving IdleStateEvent [incubator-celeborn]
cxzl25 opened a new pull request, #2329:
URL: https://github.com/apache/incubator-celeborn/pull/2329
### What changes were proposed in this pull request?
When an `IdleStateEvent ` is received, the configuration items of the corresponding module are output.
### Why are the changes needed?
Now that the `IdleStateEvent` event is received, only the timeout time is output, but the corresponding configuration items are not output.
```
24/02/26 04:12:08,062 [data-client-5-8] ERROR TransportChannelHandler: Connection to /XXX:YYY has been quiet for 240000 ms while there are outstanding requests.
```
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
GA
--
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-1288] Prompt configuration items when receiving IdleStateEvent [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #2329:
URL: https://github.com/apache/incubator-celeborn/pull/2329#discussion_r1502353082
##########
common/src/main/java/org/apache/celeborn/common/network/TransportContext.java:
##########
@@ -177,7 +177,8 @@ private TransportChannelHandler createChannelHandler(
conf.connectionTimeoutMs(),
closeIdleConnections,
enableHeartbeat,
- conf.clientHeartbeatInterval());
+ conf.clientHeartbeatInterval(),
+ this);
Review Comment:
You are right, but the changes here refer to the Spark source code, which exposed `numRegisteredConnections` metrics in [SPARK-25642](https://issues.apache.org/jira/browse/SPARK-25642).
Maybe celeborn can do the same in the future.
--
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-1288] Prompt configuration items when receiving IdleStateEvent [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #2329: [CELEBORN-1288] Prompt configuration items when receiving IdleStateEvent
URL: https://github.com/apache/incubator-celeborn/pull/2329
--
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-1288] Prompt configuration items when receiving IdleStateEvent [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #2329:
URL: https://github.com/apache/incubator-celeborn/pull/2329#discussion_r1502013907
##########
common/src/main/java/org/apache/celeborn/common/network/server/TransportChannelHandler.java:
##########
@@ -170,9 +174,11 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) {
String address = NettyUtils.getRemoteAddress(ctx.channel());
logger.error(
"Connection to {} has been quiet for {} ms while there are outstanding "
- + "requests.",
+ + "requests. Assuming connection is dead; please adjust"
+ + " celeborn.{}.io.connectionTimeout if this is wrong.",
Review Comment:
```suggestion
+ "requests. Assuming the connection is dead; consider adjusting "
+ "celeborn.{}.io.connectionTimeout if this is wrong.",
```
--
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-1288] Prompt configuration items when receiving IdleStateEvent [incubator-celeborn]
Posted by "jiaoqingbo (via GitHub)" <gi...@apache.org>.
jiaoqingbo commented on code in PR #2329:
URL: https://github.com/apache/incubator-celeborn/pull/2329#discussion_r1502078542
##########
common/src/main/java/org/apache/celeborn/common/network/TransportContext.java:
##########
@@ -177,7 +177,8 @@ private TransportChannelHandler createChannelHandler(
conf.connectionTimeoutMs(),
closeIdleConnections,
enableHeartbeat,
- conf.clientHeartbeatInterval());
+ conf.clientHeartbeatInterval(),
+ this);
Review Comment:
this -> conf.getModuleName() ? May be better if only modulename is used in the log？
--
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-1288] Prompt configuration items when receiving IdleStateEvent [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #2329:
URL: https://github.com/apache/incubator-celeborn/pull/2329#issuecomment-1964402405
Merged to main/0.4
--
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-1288] Prompt configuration items when receiving IdleStateEvent [incubator-celeborn]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #2329:
URL: https://github.com/apache/incubator-celeborn/pull/2329#issuecomment-1963263853
## [Codecov](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2329?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:
> Project coverage is 48.70%. Comparing base [(`621a719`)](https://app.codecov.io/gh/apache/incubator-celeborn/commit/621a719d8d417bd35c9f687dd05602e57e0c96f6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`196ddc0`)](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2329?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 7 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #2329 +/- ##
==========================================
- Coverage 48.73% 48.70% -0.03%
==========================================
Files 207 207
Lines 12978 12941 -37
Branches 1117 1111 -6
==========================================
- Hits 6324 6302 -22
+ Misses 6246 6232 -14
+ Partials 408 407 -1
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-celeborn/pull/2329?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-1288] Prompt configuration items when receiving IdleStateEvent [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #2329:
URL: https://github.com/apache/incubator-celeborn/pull/2329#discussion_r1502013470
##########
common/src/main/java/org/apache/celeborn/common/network/server/TransportChannelHandler.java:
##########
@@ -170,9 +174,11 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) {
String address = NettyUtils.getRemoteAddress(ctx.channel());
logger.error(
"Connection to {} has been quiet for {} ms while there are outstanding "
- + "requests.",
+ + "requests. Assuming connection is dead; please adjust"
+ + " celeborn.{}.io.connectionTimeout if this is wrong.",
address,
- requestTimeoutNs / 1000 / 1000);
+ requestTimeoutNs / 1000 / 1000,
Review Comment:
can we print it in a human-readable format?
--
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