[Fix-17139][Common] Fix rollViewLogLines IndexOutOfBoundsExceptionindex #17140
Conversation
# Conflicts: # dolphinscheduler-alert/dolphinscheduler-alert-server/src/main/bin/jvm_args_env.sh # dolphinscheduler-api/src/main/bin/jvm_args_env.sh # dolphinscheduler-master/src/main/bin/jvm_args_env.sh # dolphinscheduler-standalone-server/src/main/bin/jvm_args_env.sh # dolphinscheduler-worker/src/main/bin/jvm_args_env.sh
deng-jeffer
changed the title
deng-jeffer
changed the title
There was a problem hiding this comment.
Pull Request Overview
This pull request addresses the IndexOutOfBoundsException in the rollViewLogLines method by modifying the way log line sizes are measured. The changes include replacing the byte-based size calculation with a character-based length measurement and updating related variable names and log messages.
Comments suppressed due to low confidence (1)
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/LogUtils.java:145
- [nitpick] The variable name 'totalLogSize' may be ambiguous since it now counts characters instead of bytes. Consider renaming it to 'totalLogCharCount' or similar to clarify its purpose.
int totalLogSize = 0;
| final int lineSize = line.length(); | ||
| if (lineSize >= MaxResponseLogSize) { | ||
| builder.append(line, 0, MaxResponseLogSize) | ||
| .append(" [this line's size ").append(lineByteSize).append(" bytes is exceed ") | ||
| .append(" [this line's size ").append(lineSize).append(" bytes is exceed ") | ||
| .append(MaxResponseLogSize).append(" bytes, so only ") | ||
| .append(MaxResponseLogSize).append(" characters are reserved for performance reasons.]") |
There was a problem hiding this comment.
Changing the size measurement from bytes to characters may lead to inaccurate log size calculations when non-ASCII characters are present. Consider retaining byte-based calculation or ensuring that the character-based approach meets the intended requirements.
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/LogUtils.java
Outdated
Show resolved
Hide resolved
SbloodyS
changed the title
…ler/common/utils/LogUtils.java Co-authored-by: Copilot <[email protected]>
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/LogUtils.java
Outdated
Show resolved
Hide resolved
…ler/common/utils/LogUtils.java Co-authored-by: Wenjun Ruan <[email protected]>
done. |
|
3 participants
Purpose of the pull request
fix #17139