Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improvement] Improve the error log message for checkBlockSendResult #1464

Closed
3 tasks done
rickyma opened this issue Jan 17, 2024 · 0 comments
Closed
3 tasks done

[Improvement] Improve the error log message for checkBlockSendResult #1464

rickyma opened this issue Jan 17, 2024 · 0 comments

Comments

@rickyma
Copy link
Contributor

rickyma commented Jan 17, 2024

Code of Conduct

Search before asking

  • I have searched in the issues and found no similar issues.

What would you like to be improved?

Currently, when method checkBlockSendResult will only print error log like below:
org.apache.uniffle.common.exception.RssSendFailedException: Send failed: Task[7173_0] failed because 72 blocks can't be sent to shuffle server.
This is not enough. I think a more detailed version would be something like below:
org.apache.uniffle.common.exception.RssSendFailedException: Send failed: Task[7173_0] failed because 72 blocks can't be sent to shuffle server: [ShuffleServerInfo{id[127.0.0.1-19977-17000], host[127.0.0.1], grpc port[19977], netty port[17000]}].

Also, I think we don't need id in ShuffleServerInfo.toString, which I think is useless.

So the final output would be:
org.apache.uniffle.common.exception.RssSendFailedException: Send failed: Task[7173_0] failed because 72 blocks can't be sent to shuffle server: [ShuffleServerInfo{host[127.0.0.1], grpc port[19977], netty port[17000]}].

This is useful when troubleshooting.

How should we improve?

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
rickyma added a commit to rickyma/incubator-uniffle that referenced this issue Jan 17, 2024
rickyma added a commit to rickyma/incubator-uniffle that referenced this issue Jan 17, 2024
rickyma added a commit to rickyma/incubator-uniffle that referenced this issue Jan 17, 2024
rickyma added a commit to rickyma/incubator-uniffle that referenced this issue Jan 17, 2024
rickyma added a commit to rickyma/incubator-uniffle that referenced this issue Jan 17, 2024
rickyma added a commit to rickyma/incubator-uniffle that referenced this issue Jan 17, 2024
zuston pushed a commit that referenced this issue Jan 18, 2024
…s fail to send (#1465)

### What changes were proposed in this pull request?

Improve the error log message for checkBlockSendResult.
As described in [#1464](#1464), it will print out the shuffle server's IP and ports.

### Why are the changes needed?

For [#1464](#1464)

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Manually.
@zuston zuston closed this as completed Jan 18, 2024
rickyma added a commit to rickyma/incubator-uniffle that referenced this issue Jan 18, 2024
rickyma added a commit to rickyma/incubator-uniffle that referenced this issue Jan 19, 2024
rickyma added a commit to rickyma/incubator-uniffle that referenced this issue Jan 19, 2024
rickyma added a commit to rickyma/incubator-uniffle that referenced this issue Jan 19, 2024
jerqi pushed a commit that referenced this issue Jan 22, 2024
…that blocks fail to send (#1473)

### What changes were proposed in this pull request?

The output of failed shuffle servers' result has not been successfully deduplicated.

### Why are the changes needed?

It's a followup PR for [#1465](#1465).

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Existing UTs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants