Skip to content

Comments

[CELEBORN-1897] Avoid calling toString for too long messages#3139

Closed
CodingCat wants to merge 1 commit intoapache:mainfrom
CodingCat:log
Closed

[CELEBORN-1897] Avoid calling toString for too long messages#3139
CodingCat wants to merge 1 commit intoapache:mainfrom
CodingCat:log

Conversation

@CodingCat
Copy link
Contributor

@CodingCat CodingCat commented Mar 9, 2025

What changes were proposed in this pull request?

we hit the issue that the ignored message is too long , and when calling toString, it applies for a too large array which is beyond the JVM's limit 

I don't think this log convey too much info, , so we could avoid calling toString to improve robustness of the applications

Why are the changes needed?

more details in JIRA

Does this PR introduce any user-facing change?

no

How was this patch tested?

prod

Copy link
Contributor

@RexXiong RexXiong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, cc @FMX

@SteNicholas SteNicholas changed the title [CELEBORN-1897] avoid calling toString for too long messages [CELEBORN-1897] Avoid calling toString for too long messages Mar 10, 2025
Copy link
Member

@SteNicholas SteNicholas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

SteNicholas pushed a commit that referenced this pull request Mar 10, 2025
### What changes were proposed in this pull request?

https://github.com/apache/celeborn/blob/196ad607cd62af83b1ace887b8eb91d548fc36ac/common/src/main/scala/org/apache/celeborn/common/rpc/netty/NettyRpcEnv.scala#L256 we hit the issue that the ignored message is too long , and when calling toString, it applies for a too large array which is beyond the JVM's limit 

I don't think this log convey too much info, , so we could avoid calling toString to improve robustness of the applications

### Why are the changes needed?

more details in JIRA

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

no

### How was this patch tested?

prod

Closes #3139 from CodingCat/log.

Authored-by: CodingCat <zhunansjtu@gmail.com>
Signed-off-by: SteNicholas <programgeek@163.com>
(cherry picked from commit 18b268d)
Signed-off-by: SteNicholas <programgeek@163.com>
@SteNicholas
Copy link
Member

Merged to main(v0.6.0) and branch-0.5(v0.5.4).

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

Successfully merging this pull request may close these issues.

3 participants