fix: -XX:HeapDumpPath doesn't ready when meet OOM #5157
Conversation
dosubot
bot
added
the
size:XS
WalkthroughIn version 2.3.0 of Apollo, a fix was introduced to ensure the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Anilople
added
area/configservice
dosubot
bot
added
the
size:XS
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (14)
CHANGES.md (2)
Line range hint
1-1: Ensure headings are surrounded by blank lines for proper markdown formatting.+ Changes by Version ================== Release Notes. +
Line range hint
8-8: Ensure lists are surrounded by blank lines for proper markdown formatting.+ * [Fix circular references on LdapAutoConfiguration](https://github.com/apolloconfig/apollo/pull/5055) * [Add comment for clusters and UI display](https://github.com/apolloconfig/apollo/pull/5072) * [Fix the issue that the length of private namespaces are mis-calculated](https://github.com/apolloconfig/apollo/pull/5078) * [apollo assembly optimization](https://github.com/apolloconfig/apollo/pull/5035) * [update the config item table column width](https://github.com/apolloconfig/apollo/pull/5131) * [sync apollo portal server config to apollo quick start server](https://github.com/apolloconfig/apollo/pull/5134) * [Fix the role permission deletion issue when appid contains '_'](https://github.com/apolloconfig/apollo/pull/5150) * [fix: -XX:HeapDumpPath doesn't ready when meet OOM](https://github.com/apolloconfig/apollo/pull/5157) +apollo-portal/src/main/scripts/startup.sh (4)
Line range hint
55-55: Replace iterating overlsoutput with globs to avoid issues with filenames containing spaces or special characters.- for i in `ls $SERVICE_NAME-*.jar 2>/dev/null` + for i in $SERVICE_NAME-*.jarAlso applies to: 136-136, 147-147
Line range hint
90-90: Ensure variables and commands are properly quoted to prevent word splitting and globbing issues.- if [ "$(uname)" == "Darwin" ]; then + if [ "$(uname)" == "Darwin" ] || [ "$(uname)" == "Linux" ]; thenAlso applies to: 92-92, 134-134, 146-146
Line range hint
101-101: Declare and assign separately to avoid masking return values.- declare -i counter=0 + declare -i counter + counter=0
[!TIP]
Codebase VerificationThe
resultvariable is assigned a value but is not used elsewhere in the script. Consider removing it to clean up the code.
- Line 59:
result=$(ps -p "$pid")Analysis chain
Line range hint
59-59: Verify the usage of theresultvariable or consider removing it if it's not used.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the `result` variable is used elsewhere in the script. rg --type bash 'result' apollo-portal/src/main/scripts/startup.shLength of output: 94
Script:
#!/bin/bash # Description: Check if the `result` variable is used elsewhere in the script. rg --type sh 'result' apollo-portal/src/main/scripts/startup.shLength of output: 94
apollo-adminservice/src/main/scripts/startup.sh (4)
Line range hint
55-55: Replace iterating overlsoutput with globs to avoid issues with filenames containing spaces or special characters.- for i in `ls $SERVICE_NAME-*.jar 2>/dev/null` + for i in $SERVICE_NAME-*.jarAlso applies to: 136-136, 147-147
Line range hint
90-90: Ensure variables and commands are properly quoted to prevent word splitting and globbing issues.- if [ "$(uname)" == "Darwin" ]; then + if [ "$(uname)" == "Darwin" ] || [ "$(uname)" == "Linux" ]; thenAlso applies to: 92-92, 134-134, 146-146
Line range hint
101-101: Declare and assign separately to avoid masking return values.- declare -i counter=0 + declare -i counter + counter=0
[!TIP]
Codebase VerificationThe
resultvariable is assigned a value but is not used elsewhere in the script. Consider removing it to clean up the code.
- Line 59:
result=$(ps -p "$pid")Analysis chain
Line range hint
59-59: Verify the usage of theresultvariable or consider removing it if it's not used.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the `result` variable is used elsewhere in the script. rg --type bash 'result' apollo-adminservice/src/main/scripts/startup.shLength of output: 100
Script:
#!/bin/bash # Description: Check if the `result` variable is used elsewhere in the script. rg --type sh 'result' apollo-adminservice/src/main/scripts/startup.shLength of output: 100
apollo-configservice/src/main/scripts/startup.sh (4)
Line range hint
55-55: Replace iterating overlsoutput with globs to avoid issues with filenames containing spaces or special characters.- for i in `ls $SERVICE_NAME-*.jar 2>/dev/null` + for i in $SERVICE_NAME-*.jarAlso applies to: 136-136, 147-147
Line range hint
90-90: Ensure variables and commands are properly quoted to prevent word splitting and globbing issues.- if [ "$(uname)" == "Darwin" ]; then + if [ "$(uname)" == "Darwin" ] || [ "$(uname)" == "Linux" ]; thenAlso applies to: 92-92, 134-134, 146-146
Line range hint
101-101: Declare and assign separately to avoid masking return values.- declare -i counter=0 + declare -i counter + counter=0
[!TIP]
Codebase VerificationThe
resultvariable is assigned a value but is not used elsewhere in thestartup.shscript. Consider removing it to clean up the code.
- Line 59:
result=$(ps -p "$pid")Analysis chain
Line range hint
59-59: Verify the usage of theresultvariable or consider removing it if it's not used.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the `result` variable is used elsewhere in the script. rg --type bash 'result' apollo-configservice/src/main/scripts/startup.shLength of output: 101
Script:
#!/bin/bash # Description: Check if the `result` variable is used elsewhere in the script. rg --type sh 'result' apollo-configservice/src/main/scripts/startup.shLength of output: 101
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- CHANGES.md (1 hunks)
- apollo-adminservice/src/main/scripts/startup.sh (1 hunks)
- apollo-configservice/src/main/scripts/startup.sh (1 hunks)
- apollo-portal/src/main/scripts/startup.sh (1 hunks)
Additional context used
Markdownlint
CHANGES.md
1-1: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
8-8: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
Shellcheck
apollo-portal/src/main/scripts/startup.sh
[warning] 55-55: Iterating over ls output is fragile. Use globs. (SC2045)
[warning] 59-59: result appears unused. Verify use (or export if used externally). (SC2034)
[warning] 90-90: Quote this to prevent word splitting. (SC2046)
[warning] 92-92: Quote this to prevent word splitting. (SC2046)
[warning] 101-101: Declare and assign separately to avoid masking return values. (SC2155)
[warning] 134-134: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. (SC2164)
[warning] 134-134: Quote this to prevent word splitting. (SC2046)
[error] 136-136: Iterating over ls output is fragile. Use globs. (SC2045)
[warning] 146-146: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. (SC2164)
[error] 147-147: Iterating over ls output is fragile. Use globs. (SC2045)
apollo-adminservice/src/main/scripts/startup.sh
[warning] 55-55: Iterating over ls output is fragile. Use globs. (SC2045)
[warning] 59-59: result appears unused. Verify use (or export if used externally). (SC2034)
[warning] 90-90: Quote this to prevent word splitting. (SC2046)
[warning] 92-92: Quote this to prevent word splitting. (SC2046)
[warning] 101-101: Declare and assign separately to avoid masking return values. (SC2155)
[warning] 134-134: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. (SC2164)
[warning] 134-134: Quote this to prevent word splitting. (SC2046)
[error] 136-136: Iterating over ls output is fragile. Use globs. (SC2045)
[warning] 146-146: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. (SC2164)
[error] 147-147: Iterating over ls output is fragile. Use globs. (SC2045)
apollo-configservice/src/main/scripts/startup.sh
[warning] 55-55: Iterating over ls output is fragile. Use globs. (SC2045)
[warning] 59-59: result appears unused. Verify use (or export if used externally). (SC2034)
[warning] 90-90: Quote this to prevent word splitting. (SC2046)
[warning] 92-92: Quote this to prevent word splitting. (SC2046)
[warning] 101-101: Declare and assign separately to avoid masking return values. (SC2155)
[warning] 134-134: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. (SC2164)
[warning] 134-134: Quote this to prevent word splitting. (SC2046)
[error] 136-136: Iterating over ls output is fragile. Use globs. (SC2045)
[warning] 146-146: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. (SC2164)
[error] 147-147: Iterating over ls output is fragile. Use globs. (SC2045)
Additional comments not posted (3)
apollo-portal/src/main/scripts/startup.sh (1)
25-26: Ensure the directory for heap dumps is created during startup.This change correctly prepares the directory for heap dumps, aligning with the PR's objective to handle OOM errors more gracefully.
apollo-adminservice/src/main/scripts/startup.sh (1)
25-26: Ensure the directory for heap dumps is created during startup.This change correctly prepares the directory for heap dumps, aligning with the PR's objective to handle OOM errors more gracefully.
apollo-configservice/src/main/scripts/startup.sh (1)
25-26: Ensure the directory for heap dumps is created during startup.This change correctly prepares the directory for heap dumps, aligning with the PR's objective to handle OOM errors more gracefully.
What's the purpose of this PR
fix scripts/
startup.shWhich issue(s) this PR fixes:
Brief changelog
When meet OOM, the .hprof file should auto dump,
but the directory not ready, so meet some error.
This pr prepare the directory for OOM when startup.
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.CHANGESlog.Summary by CodeRabbit
Bug Fixes
-XX:HeapDumpPathwas not ready when encountering an OutOfMemory error.Chores
HeapDumpOnOutOfMemoryErrordirectory under the existing log directory.