feat(diagnostics): add memory-analysis.sh for sizing diagnostics#41816
Open
wyattwalter wants to merge 4 commits into
Open
feat(diagnostics): add memory-analysis.sh for sizing diagnostics#41816wyattwalter wants to merge 4 commits into
wyattwalter wants to merge 4 commits into
Conversation
Self-contained shell script that prints a single-page memory report from inside the Appsmith container: system/cgroup RAM, per-role RSS, JVM flag origins (ergonomic vs. explicit), MongoDB WiredTiger cache state + data-dir size, live usage vs. configured budget, and a verdict (OK / TIGHT / AT RISK). Wired into diagnostics.sh so every support tarball includes the report alongside the existing heap/thread dumps. Optional --threads flag adds a single-snapshot thread breakdown (state histogram, name-prefix groups, hot stacks, deadlock check). --threads-sample takes two snapshots N seconds apart to surface threads that stayed RUNNABLE on the same frame.
Contributor
WalkthroughAdds a new ChangesMemory analysis and diagnostics integration
Sequence Diagram(s)sequenceDiagram
participant Diagnostics as diagnostics.sh
participant MemoryScript as memory-analysis.sh
participant CGroupFS as cgroupFS
participant JCmd as jcmd
participant MongoClient as mongosh/mongo
Diagnostics->>MemoryScript: run --threads, capture output
MemoryScript->>CGroupFS: read memory limits/usage or /proc/meminfo
MemoryScript->>JCmd: jcmd (GC, Thread.print) [optional]
MemoryScript->>MongoClient: query wiredTiger.cache via APPSMITH_DB_URL [optional]
MemoryScript->>Diagnostics: emit snapshot, verdict, exit 0
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
deploy/docker/fs/opt/appsmith/memory-analysis.sh (2)
33-49: 💤 Low valueArgument parsing is forgiving to a fault.
Two small ergonomics gaps:
- Unknown flags fall through silently (no
*)arm), so a typo like--no-mongoolooks accepted but does nothing.--threads-sample=0keepsWITH_THREADS=1while leavingSAMPLE_SECS=0, which quietly disables stuck-thread sampling but still runs the single-snapshot section. Treating0like any other sub-3 value (clamp to 3) would match the documented "3-60s" contract.🛠 Proposed fix
--threads-sample=*) WITH_THREADS=1; SAMPLE_SECS="${arg#*=}" ;; -h|--help) sed -n '2,24p' "$0"; exit 0 ;; + *) + echo "memory-analysis.sh: unknown argument '$arg' (try --help)" >&2 + exit 2 ;; esac done # Clamp sample window to a sane range. if [[ "$SAMPLE_SECS" =~ ^[0-9]+$ ]]; then - [[ "$SAMPLE_SECS" -gt 0 && "$SAMPLE_SECS" -lt 3 ]] && SAMPLE_SECS=3 + [[ "$SAMPLE_SECS" -lt 3 ]] && SAMPLE_SECS=3 [[ "$SAMPLE_SECS" -gt 60 ]] && SAMPLE_SECS=60 else SAMPLE_SECS=10 fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh` around lines 33 - 49, Add a default case to the argument-parsing case block to detect unknown flags and fail or warn (e.g., a *) arm that prints usage and exits or sets an error flag, so typos like --no-mongoo don't silently pass; also tighten the SAMPLE_SECS clamp logic used with WITH_THREADS so that values of 0 are treated as below the minimum (i.e., treat any numeric SAMPLE_SECS < 3 as SAMPLE_SECS=3 and still set WITH_THREADS=1) by changing the numeric checks around SAMPLE_SECS, referencing the existing variables/constructs SKIP_MONGO, WITH_THREADS, SAMPLE_SECS and the case "$arg" in parsing logic to locate and update the behavior.
494-512: ⚡ Quick winAdd a short server-selection timeout on the WT cache probe.
mongosh/mongodefaults to ~30s server-selection. WhenAPPSMITH_DB_URLpoints at an unreachable external host, this section (anddiagnostics.shby extension) will pause for the full timeout before falling through. AddingserverSelectionTimeoutMS=5000&connectTimeoutMS=5000to the connection URL keeps the probe fast and predictable.🛠 Proposed fix
- url="${url:-mongodb://localhost:27017/appsmith?replicaSet=mr1}" + url="${url:-mongodb://localhost:27017/appsmith?replicaSet=mr1}" + # Keep the probe snappy when an external Mongo is unreachable. + if [[ "$url" == *\?* ]]; then + probe_url="${url}&serverSelectionTimeoutMS=5000&connectTimeoutMS=5000" + else + probe_url="${url}?serverSelectionTimeoutMS=5000&connectTimeoutMS=5000" + fi # Hide creds when echoing - safe_url=$(sed -E 's#(mongodb(\+srv)?://)[^@/]+@#\1***:***@#' <<<"$url") + safe_url=$(sed -E 's#(mongodb(\+srv)?://)[^@/]+@#\1***:***@#' <<<"$probe_url") echo " URL: $safe_url (via $mclient)" js='var s=db.serverStatus().wiredTiger.cache; print("WT_MAX="+s["maximum bytes configured"]); print("WT_USED="+s["bytes currently in the cache"]);' - out=$("$mclient" --quiet "$url" --eval "$js" 2>&1 || true) + out=$("$mclient" --quiet "$probe_url" --eval "$js" 2>&1 || true)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh` around lines 494 - 512, The WT cache probe can hang on default 30s server-selection; update the probe to add a short server-selection and connect timeout by passing serverSelectionTimeoutMS=5000&connectTimeoutMS=5000 to the Mongo URL used for the probe: when building the probe invocation that uses js and calls out=$("$mclient" --quiet "$url" --eval "$js" ...), construct a probe-specific URL (e.g. probe_url from url) that appends the query params (taking care of existing query string separator ? vs &) and use probe_url in the mclient call so wt_max_bytes/wt_used_bytes parsing remains unchanged but the probe completes quickly on unreachable hosts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh`:
- Around line 422-428: The sed substitution never matches because it expects the
pool suffix at EOL instead of before the tab-separated frame; update the sed in
the pipeline that processes "$stuck" so it strips the "-N[-M]" suffix only on
the name field before the tab—for example change the sed expression used in the
pipe (the line starting with: echo "$stuck" | sed -E ...) to match the suffix
followed by a tab and replace it with the name plus the tab (e.g. use a pattern
like 's/^([^\t]+)-[0-9]+(-[0-9]+)*\t/\1\t/' so pool-1, pool-2 collapse into a
single name for grouping).
---
Nitpick comments:
In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh`:
- Around line 33-49: Add a default case to the argument-parsing case block to
detect unknown flags and fail or warn (e.g., a *) arm that prints usage and
exits or sets an error flag, so typos like --no-mongoo don't silently pass; also
tighten the SAMPLE_SECS clamp logic used with WITH_THREADS so that values of 0
are treated as below the minimum (i.e., treat any numeric SAMPLE_SECS < 3 as
SAMPLE_SECS=3 and still set WITH_THREADS=1) by changing the numeric checks
around SAMPLE_SECS, referencing the existing variables/constructs SKIP_MONGO,
WITH_THREADS, SAMPLE_SECS and the case "$arg" in parsing logic to locate and
update the behavior.
- Around line 494-512: The WT cache probe can hang on default 30s
server-selection; update the probe to add a short server-selection and connect
timeout by passing serverSelectionTimeoutMS=5000&connectTimeoutMS=5000 to the
Mongo URL used for the probe: when building the probe invocation that uses js
and calls out=$("$mclient" --quiet "$url" --eval "$js" ...), construct a
probe-specific URL (e.g. probe_url from url) that appends the query params
(taking care of existing query string separator ? vs &) and use probe_url in the
mclient call so wt_max_bytes/wt_used_bytes parsing remains unchanged but the
probe completes quickly on unreachable hosts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5d1a7adf-9441-4ba6-bfd8-a2d70d28addb
📒 Files selected for processing (2)
deploy/docker/fs/opt/appsmith/diagnostics.shdeploy/docker/fs/opt/appsmith/memory-analysis.sh
The pattern `(-[0-9]+(-[0-9]+)*)$` never matched because the suffix it's stripping lives between the thread name and the top frame (separated by a tab), not at end-of-line. Result: pool members like boundedElastic-1, boundedElastic-2 never collapsed into a single grouping row. Anchor on \t instead so the trailing -N(-M)* is stripped from the name field before the tab.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
deploy/docker/fs/opt/appsmith/memory-analysis.sh (2)
33-42: 💤 Low valueUnknown flags are silently swallowed.
The
caseblock has no default arm, so a typo like--thredsis accepted, parsed as a no-op, and the report runs in default mode — making it easy to think you enabled sampling when you didn't. A one-liner default arm with a friendly hint (and stillexit 0to honor the "always succeed" contract) goes a long way:🛠 Suggested guard
--threads-sample=*) WITH_THREADS=1; SAMPLE_SECS="${arg#*=}" ;; -h|--help) sed -n '2,24p' "$0"; exit 0 ;; + *) + echo "memory-analysis.sh: unknown option '$arg' — see --help" >&2 ;; esac🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh` around lines 33 - 42, The case block that parses arguments in memory-analysis.sh currently swallows unknown flags (so typos like --threds are ignored); add a default (*) case to that case statement which prints a friendly hint mentioning the unrecognized flag and usage (e.g., echo "Unknown option: $arg — see --help"), and then exit 0 to preserve the script's "always succeed" behavior; ensure this default arm is placed alongside the existing arms that set SKIP_MONGO, WITH_THREADS, and SAMPLE_SECS so typos to those flags are detected and reported.
149-189: 💤 Low value
role_nis populated but never read — dead bookkeeping.
role_n[$role]=$nis set on line 180 and never consulted anywhere else in the script (the per-row count is printed inline asn=%don line 182). Shellcheck's SC2034 is correct here; therole_pattern_*warnings are false positives because of the indirect${!var}expansion. Drop the array and itsdeclareif you'd like to keep the file quiet under linters:🧹 Suggested cleanup
declare -A role_kb -declare -A role_n declare -a role_order=(java rts mongod postgres redis keycloak temporal caddy supervisord) @@ kb=$(sum_rss_kb "$pat") n=$(count_procs "$pat") role_kb[$role]=$kb - role_n[$role]=$n if [[ "$kb" -gt 0 ]]; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh` around lines 149 - 189, The role_n associative array is unused (populated via role_n[$role]=$n) and triggers ShellCheck SC2034; remove the dead bookkeeping by deleting the declare -A role_n line and the assignment role_n[$role]=$n, and keep printing the per-row count using the existing n variable (from count_procs "$pat") so functionality and the inline printf 'n=%d' remain unchanged; references to role_n, declare -A role_n, role_n[$role]=$n, and count_procs appear in this loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh`:
- Around line 33-49: The --threads-sample parsing currently sets WITH_THREADS=1
and assigns SAMPLE_SECS before validation which lets values like 0 or negatives
disable sampling silently; change the handling so that when parsing
--threads-sample=* you validate SAMPLE_SECS immediately: if it is not an
integer, set SAMPLE_SECS=10; if it is an integer and SAMPLE_SECS < 3 (including
0 or negatives) set SAMPLE_SECS=3; if > 60 set SAMPLE_SECS=60; KEEP
WITH_THREADS=1 for any --threads-sample invocation so the flag's intent is
honored and the later stall/stack sampling code (which checks SAMPLE_SECS and
WITH_THREADS) behaves predictably.
- Line 200: The pgrep call that sets java_pid uses a PCRE-style escape `\s*`
which POSIX ERE doesn't support, so update the pgrep -f pattern used to find the
java process (the line assigning java_pid) to use a POSIX-compliant whitespace
class or literal space, e.g. replace `-jar\s*server\.jar` with a pattern using
`[[:space:]]*` or a single space (e.g. `-jar[[:space:]]*server\.jar` or `-jar
server\.jar`) so the process lookup reliably matches the java -jar server.jar
invocation.
---
Nitpick comments:
In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh`:
- Around line 33-42: The case block that parses arguments in memory-analysis.sh
currently swallows unknown flags (so typos like --threds are ignored); add a
default (*) case to that case statement which prints a friendly hint mentioning
the unrecognized flag and usage (e.g., echo "Unknown option: $arg — see
--help"), and then exit 0 to preserve the script's "always succeed" behavior;
ensure this default arm is placed alongside the existing arms that set
SKIP_MONGO, WITH_THREADS, and SAMPLE_SECS so typos to those flags are detected
and reported.
- Around line 149-189: The role_n associative array is unused (populated via
role_n[$role]=$n) and triggers ShellCheck SC2034; remove the dead bookkeeping by
deleting the declare -A role_n line and the assignment role_n[$role]=$n, and
keep printing the per-row count using the existing n variable (from count_procs
"$pat") so functionality and the inline printf 'n=%d' remain unchanged;
references to role_n, declare -A role_n, role_n[$role]=$n, and count_procs
appear in this loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 43eb5e80-1e9c-4ebc-9e31-775a726ded2c
📒 Files selected for processing (1)
deploy/docker/fs/opt/appsmith/memory-analysis.sh
| # ---------- 3. JVM ---------------------------------------------------------- | ||
|
|
||
| hdr "JVM (Appsmith server)" | ||
| java_pid=$(pgrep -f -- "-jar\s*server\.jar" | head -1 || true) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does pgrep from procps-ng support \s as a whitespace shorthand in its regex, or does it only support POSIX extended regular expressions?
💡 Result:
pgrep from procps-ng supports only POSIX Extended Regular Expressions (ERE), not Perl-compatible shorthands like \s for whitespace. It uses the standard regcomp function with the REG_EXTENDED flag, which implements POSIX ERE where \s is not defined.[1][2][3][4] POSIX ERE lacks \s; use [[:space:]] instead for whitespace matching.[4][5] No evidence of PCRE or extended regex support in procps-ng pgrep source code or documentation.[6][7]
Citations:
- 1: https://github.com/mmalecki/procps/blob/master/pgrep.c
- 2: https://gitlab.com/procps-ng/procps/-/blob/dcb6914f11406a13972636b08b7e26fdafe9efc9/pgrep.c
- 3: https://man7.org/linux/man-pages/man1/pgrep.1.html
- 4: https://www.man7.org/linux/man-pages/man3/regcomp.3.html
- 5: https://pubs.opengroup.org/onlinepubs/9799919799/functions/regcomp.html
- 6: https://github.com/procps-ng/procps
- 7: https://gitlab.com/procps-ng/procps/-/blob/f12277c74d591245767d77badb6bb6af91335656/pgrep.1
🏁 Script executed:
#!/bin/bash
# Test pgrep behavior with \s vs [[:space:]]
# Simulate the Java process running appsmith server
# Create test scenarios
echo "=== Testing pgrep patterns ==="
# Test 1: Check if pgrep is available and test with a sample command
if command -v pgrep &> /dev/null; then
# Create a test process by running a background command that matches the pattern
# Simulate: java -jar /opt/appsmith/server/server.jar
bash -c 'sleep 300 &' &
# Get the PID for testing
TEST_PID=$!
echo "Test with typical Java command format: java -jar /opt/appsmith/server/server.jar"
# Test pattern 1: Original pattern with \s
echo -e "\nPattern 1 (with \\s): -jar\\s*server\\.jar"
pgrep -f -- "-jar\s*server\.jar" 2>&1 || echo "No match found"
# Test pattern 2: Corrected pattern with [[:space:]]
echo -e "\nPattern 2 (with [[:space:]]): -jar[[:space:]]+server\\.jar"
pgrep -f -- "-jar[[:space:]]+server\.jar" 2>&1 || echo "No match found"
# Clean up
kill $TEST_PID 2>/dev/null
else
echo "pgrep not available"
fi
# Test 2: Check procps-ng source to understand \s handling
echo -e "\n=== Checking regex handling in procps-ng ==="
# Let's look at the pgrep man page for regex information
if command -v man &> /dev/null; then
man pgrep 2>/dev/null | grep -A 10 "regex\|expression" | head -20
fiRepository: appsmithorg/appsmith
Length of output: 161
🏁 Script executed:
#!/bin/bash
# Find and examine the memory-analysis.sh file
find . -name "memory-analysis.sh" -type f 2>/dev/null | head -5
# If found, check the specific line context
if [ -f "deploy/docker/fs/opt/appsmith/memory-analysis.sh" ]; then
echo "=== File found. Examining lines around 200 ==="
sed -n '195,205p' deploy/docker/fs/opt/appsmith/memory-analysis.sh | cat -n
fiRepository: appsmithorg/appsmith
Length of output: 749
pgrep -f regex pattern uses \s, which isn't recognized in POSIX ERE — server.jar discovery will fail.
pgrep from procps-ng uses POSIX extended regex (via regcomp(3) with REG_EXTENDED), where \s is not a valid escape sequence. The pattern -jar\s*server\.jar will be interpreted as literal -jar followed by the character s repeated zero or more times, then server.jar. This won't match the actual command line format java … -jar /opt/appsmith/server/server.jar (with a space between -jar and the path), causing the process lookup to fail and the entire JVM section to be skipped.
Fix — use POSIX-compliant whitespace class
-java_pid=$(pgrep -f -- "-jar\s*server\.jar" | head -1 || true)
+java_pid=$(pgrep -f -- "-jar[[:space:]]+server\.jar" | head -1 || true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| java_pid=$(pgrep -f -- "-jar\s*server\.jar" | head -1 || true) | |
| java_pid=$(pgrep -f -- "-jar[[:space:]]+server\.jar" | head -1 || true) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh` at line 200, The pgrep call
that sets java_pid uses a PCRE-style escape `\s*` which POSIX ERE doesn't
support, so update the pgrep -f pattern used to find the java process (the line
assigning java_pid) to use a POSIX-compliant whitespace class or literal space,
e.g. replace `-jar\s*server\.jar` with a pattern using `[[:space:]]*` or a
single space (e.g. `-jar[[:space:]]*server\.jar` or `-jar server\.jar`) so the
process lookup reliably matches the java -jar server.jar invocation.
Previous clamp only nudged 1/2 up to 3, leaving 0 untouched. A user passing --threads-sample=0 then got WITH_THREADS=1 with SAMPLE_SECS=0, which silently skipped the sampling section while still printing the basic threads view. Honor the flag's intent by clamping any in-range integer below 3 up to 3.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
deploy/docker/fs/opt/appsmith/memory-analysis.sh (1)
200-200:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a POSIX ERE here or JVM discovery can disappear.
Line 200 still uses
\s, whichpgrepdoes not treat as whitespace. On procps-based images this can miss the Appsmith JVM entirely, and the JVM section, thread diagnostics, and/proc/<pid>/environMongo URL lookup all degrade silently.💡 Minimal fix
-java_pid=$(pgrep -f -- "-jar\s*server\.jar" | head -1 || true) +java_pid=$(pgrep -f -- "-jar[[:space:]]+(/opt/appsmith/server/)?server\.jar" | head -1 || true)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh` at line 200, The pgrep pattern used to set java_pid currently relies on the Perl-style \s which pgrep doesn't recognize; update the pgrep invocation that assigns java_pid to use a POSIX ERE whitespace class (e.g., replace \s with [[:space:]] or an equivalent POSIX ERE) so the "-jar ... server.jar" match reliably finds the JVM on procps-based images; adjust only the pattern used where java_pid is defined to ensure downstream JVM diagnostics and /proc/<pid>/environ-based Mongo lookup work.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh`:
- Around line 32-49: The problem is SAMPLE_SECS is defaulted to 0 but the
numeric clamping unconditionally raises any numeric value to >=3, so invoking
--threads (which sets WITH_THREADS but leaves SAMPLE_SECS as 0) ends up treated
as a 3s sample; update the clamp logic so it only enforces the 3–60 bounds when
SAMPLE_SECS is a non-zero numeric value. Concretely, change the block that tests
SAMPLE_SECS (the existing regex check and subsequent [[ "$SAMPLE_SECS" -lt 3 ]]
and -gt 60 checks) to first verify SAMPLE_SECS is numeric and not equal to 0
(e.g., if [[ "$SAMPLE_SECS" =~ ^[0-9]+$ && "$SAMPLE_SECS" -ne 0 ]]; then apply
the min/max clamps; else set SAMPLE_SECS=0 or the current default for
non-numeric flags), so --threads (WITH_THREADS) leaves SAMPLE_SECS at 0 while
--threads-sample still sets and clamps its explicit value.
- Around line 494-511: The script currently redacts credentials for the
connection URL into safe_url but later prints raw mongosh/mongo client output
stored in out (from "$mclient" --eval ...), which may contain the full URI;
modify the code that prints the probe failure line to sanitize out first using
the same redaction regex (for example apply sed -E
's#(mongodb(\+srv)?://)[^@/]+@#\1***:***@#' to $out) and then echo the sanitized
variable (e.g. safe_out) instead of raw $out when building the "(probe failed) —
..." message; update references to out in this block only so other logic
(wt_max_bytes parsing) still operates on the original out if needed.
---
Duplicate comments:
In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh`:
- Line 200: The pgrep pattern used to set java_pid currently relies on the
Perl-style \s which pgrep doesn't recognize; update the pgrep invocation that
assigns java_pid to use a POSIX ERE whitespace class (e.g., replace \s with
[[:space:]] or an equivalent POSIX ERE) so the "-jar ... server.jar" match
reliably finds the JVM on procps-based images; adjust only the pattern used
where java_pid is defined to ensure downstream JVM diagnostics and
/proc/<pid>/environ-based Mongo lookup work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a98a8a8b-ec58-4c9d-adcf-9c39770ddf68
📒 Files selected for processing (1)
deploy/docker/fs/opt/appsmith/memory-analysis.sh
…rors Two issues caught by review: 1. Last commit's unconditional <3 clamp regressed --threads by clamping the default SAMPLE_SECS=0 up to 3, which then triggered the stuck- thread sampling section. Introduce WITH_THREAD_SAMPLE to distinguish "sampling explicitly requested" from "default value," and gate the clamp on it. Plain --threads now correctly leaves SAMPLE_SECS=0 and skips sampling. 2. The Mongo probe-failure line printed raw mongosh/mongo error output, which can include the connection string with credentials. Apply the same URI-redaction sed to a safe_out variable used only for display.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
deploy/docker/fs/opt/appsmith/memory-analysis.sh (1)
205-205:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a POSIX-compliant
pgreppattern for the JVM lookup.Line 205 uses
\s, whichpgrep -fdoes not treat as whitespace, and it also requiresserver.jarto appear immediately after-jar. That can miss a normal-jar /path/to/server.jarargv and skip the entire JVM section.Suggested fix
-java_pid=$(pgrep -f -- "-jar\s*server\.jar" | head -1 || true) +java_pid=$(pgrep -f -- "-jar[[:space:]]+.*server\.jar" | head -1 || true)#!/bin/bash set -u bash -c 'exec -a "java -jar /opt/appsmith/server/server.jar" sleep 20' & pid=$! trap 'kill "$pid" 2>/dev/null || true' EXIT sleep 1 echo "argv:" ps -p "$pid" -o args= echo echo "Current pattern match?" if pgrep -f -- '-jar\s*server\.jar' | grep -qx "$pid"; then echo "matched" else echo "not matched" fi echo echo "POSIX-compliant pattern match?" if pgrep -f -- '-jar[[:space:]]+.*server\.jar' | grep -qx "$pid"; then echo "matched" else echo "not matched" fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh` at line 205, The pgrep pattern used to set java_pid (pgrep -f -- "-jar\s*server\.jar") is not POSIX-compliant and misses cases like "-jar /path/to/server.jar"; update the pattern used in the java_pid assignment to a POSIX-compatible regex such as '-jar[[:space:]]+.*server\.jar' so pgrep -f matches a -jar followed by whitespace and any path to server.jar; modify the java_pid line (the pgrep invocation that sets java_pid) to use this new pattern and ensure the pattern is quoted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@deploy/docker/fs/opt/appsmith/memory-analysis.sh`:
- Line 205: The pgrep pattern used to set java_pid (pgrep -f --
"-jar\s*server\.jar") is not POSIX-compliant and misses cases like "-jar
/path/to/server.jar"; update the pattern used in the java_pid assignment to a
POSIX-compatible regex such as '-jar[[:space:]]+.*server\.jar' so pgrep -f
matches a -jar followed by whitespace and any path to server.jar; modify the
java_pid line (the pgrep invocation that sets java_pid) to use this new pattern
and ensure the pattern is quoted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b92b3996-636f-49e0-9c43-dbcbceb06a84
📒 Files selected for processing (1)
deploy/docker/fs/opt/appsmith/memory-analysis.sh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
memory-analysis.sh— a single-page memory diagnostic that runs inside the Appsmith container. Reports system/cgroup RAM, per-role RSS, JVM flag origins (ergonomic vs. explicit), MongoDB WiredTiger cache state + data-dir size, live usage vs. configured budget, and a verdict (OK / TIGHT / AT RISK).diagnostics.shso every support tarball includes a readable memory report alongside the existing heap/thread dumps.How to use
The script is also invoked from
diagnostics.sh --threads, so the report ends up atmemory-analysis.txtinside the support tarball.Test plan
Summary by CodeRabbit
New Features
Chores
Warning
Tests have not run on the HEAD 4055d63 yet
Thu, 14 May 2026 20:14:06 UTC