-
Notifications
You must be signed in to change notification settings - Fork 60
SCHED-1897: Add idle node memory health check #2773
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
Changes from all commits
4649ddc
0bb7618
9e0cbb1
57ae072
1f3aa8f
5bf949d
277f99b
a5c2c10
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| node_name="${SLURMD_NODENAME:-unknown}" | ||
| node_real_memory_bytes="${CHECKS_NODE_REAL_MEM_BYTES:-}" | ||
|
|
||
| echo "[$(date)] Check memory usage when node ${node_name} is idle" | ||
| echo "Slurm RealMemory input: ${node_real_memory_bytes:-<unavailable>} bytes" | ||
| echo "Idle state source: local 'scontrol listjobs --json' (no controller RPC)" | ||
| echo "Idle state rule: an empty JSON .jobs array means the node has no local jobs" | ||
|
|
||
| if listjobs_output="$(scontrol listjobs --json 2>&1)"; then | ||
| listjobs_rc=0 | ||
| else | ||
| listjobs_rc=$? | ||
| fi | ||
|
|
||
| echo "scontrol listjobs --json exit code: ${listjobs_rc}" | ||
| echo "scontrol listjobs --json output: ${listjobs_output:-<empty>}" | ||
|
|
||
| if (( listjobs_rc != 0 )); then | ||
| echo "Could not determine whether the node is idle because 'scontrol listjobs --json' failed; skipping memory validation" >&2 | ||
| exit 0 | ||
| fi | ||
|
|
||
| if ! local_job_count="$( | ||
| jq -er ' | ||
| if (.jobs | type) == "array" then | ||
| .jobs | length | ||
| else | ||
| error(".jobs must be an array") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if the next version of Slurm returns not |
||
| end | ||
| ' <<<"${listjobs_output}" 2>/dev/null | ||
| )"; then | ||
| echo "Could not determine whether the node is idle because 'scontrol listjobs --json' returned invalid job data; skipping memory validation" >&2 | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "Local Slurm job count from JSON .jobs array: ${local_job_count}" | ||
| if (( local_job_count == 0 )); then | ||
| node_is_idle=true | ||
| echo "The JSON .jobs array is empty; treating the node as idle" | ||
| else | ||
| node_is_idle=false | ||
| echo "The JSON .jobs array contains local jobs; treating the node as non-idle" | ||
| fi | ||
|
|
||
| echo "Node is idle: ${node_is_idle}" | ||
| if [[ "${node_is_idle}" != "true" ]]; then | ||
| echo "Node has local jobs; skipping memory validation" | ||
| exit 0 | ||
| fi | ||
|
|
||
| if ! [[ "${node_real_memory_bytes}" =~ ^[0-9]+$ ]] || [[ "${node_real_memory_bytes}" == "0" ]]; then | ||
| echo "Invalid or unavailable Slurm RealMemory '${node_real_memory_bytes:-<unavailable>}'; expected a positive byte count, skipping memory validation" >&2 | ||
| exit 0 | ||
| fi | ||
|
|
||
| if ! free_bytes_output="$(LC_ALL=C free -b 2>&1)"; then | ||
| echo "Could not read local memory information with 'free -b'; skipping memory validation" >&2 | ||
| exit 0 | ||
| fi | ||
|
|
||
| memory_values="$(awk '/^Mem:/ { print $2, $7; exit }' <<<"${free_bytes_output}")" | ||
| read -r mem_total_bytes mem_available_bytes <<<"${memory_values}" | ||
|
|
||
| if ! [[ "${mem_total_bytes:-}" =~ ^[0-9]+$ ]] || | ||
| ! [[ "${mem_available_bytes:-}" =~ ^[0-9]+$ ]] || | ||
| (( mem_available_bytes > mem_total_bytes )); then | ||
| echo "Could not determine valid total and available memory from 'free -b'; skipping memory validation" >&2 | ||
| exit 0 | ||
| fi | ||
|
|
||
| if (( node_real_memory_bytes > mem_total_bytes )); then | ||
| echo "Slurm RealMemory ${node_real_memory_bytes} bytes exceeds MemTotal ${mem_total_bytes} bytes; skipping memory validation" >&2 | ||
| exit 0 | ||
| fi | ||
|
|
||
| mem_available_gb="$(awk -v bytes="${mem_available_bytes}" 'BEGIN { printf "%.2f", bytes / 1000000000 }')" | ||
| node_real_memory_gb="$(awk -v bytes="${node_real_memory_bytes}" 'BEGIN { printf "%.2f", bytes / 1000000000 }')" | ||
|
|
||
| echo "Memory source: local 'free'" | ||
| echo "Memory comparison: available=${mem_available_gb} GB (${mem_available_bytes} bytes), Slurm RealMemory=${node_real_memory_gb} GB (${node_real_memory_bytes} bytes)" | ||
| echo "Memory snapshot (free -hw):" | ||
| if ! LC_ALL=C free -hw; then | ||
| echo "Could not print the human-readable memory snapshot with 'free -hw'" >&2 | ||
| fi | ||
|
|
||
| if (( mem_available_bytes < node_real_memory_bytes )); then | ||
|
fabrizio2210 marked this conversation as resolved.
|
||
| echo "available memory ${mem_available_gb} GB < configured ${node_real_memory_gb} GB; stop leftover processes or reboot" >&3 | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "Idle node leaves enough memory available for Slurm RealMemory" | ||
| exit 0 | ||
|
fabrizio2210 marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| { | ||
|
fabrizio2210 marked this conversation as resolved.
|
||
| "name": "idle_mem_used", | ||
| "command": "./idle_mem_used.drain.sh", | ||
| "platforms": ["any"], | ||
| "skip_for_cpu_jobs": false, | ||
| "skip_for_partial_gpu_jobs": false, | ||
| "contexts": ["hc_program"], | ||
| "node_states": ["any"], | ||
| "on_fail": "drain", | ||
| "on_ok": "none", | ||
| "reason_base": "[user_problem] $name", | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rdjjke We're introducing a new drain reason and it is fine. But what about the old |
||
| "reason_append_details": true, | ||
| "run_in_jail": false, | ||
| "log": "slurm_scripts/$worker.$name.drain.$context.out", | ||
| "need_env": ["CHECKS_NODE_REAL_MEM_BYTES"] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| # This recovery check must fail closed: only a fully validated healthy result | ||
| # may allow check_runner to undrain the node. | ||
| node_name="${SLURMD_NODENAME:-unknown}" | ||
| node_real_memory_bytes="${CHECKS_NODE_REAL_MEM_BYTES:-}" | ||
|
|
||
| echo "[$(date)] Check whether memory usage has recovered on drained node ${node_name}" | ||
| echo "Slurm RealMemory input: ${node_real_memory_bytes:-<unavailable>} bytes" | ||
| echo "Node eligibility source: this check is scheduled only for drained nodes" | ||
|
|
||
| if ! [[ "${node_real_memory_bytes}" =~ ^[0-9]+$ ]] || [[ "${node_real_memory_bytes}" == "0" ]]; then | ||
| echo "Invalid or unavailable Slurm RealMemory '${node_real_memory_bytes:-<unavailable>}'; keeping node drained" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! free_bytes_output="$(LC_ALL=C free -b 2>&1)"; then | ||
| echo "Could not read local memory information with 'free -b'; keeping node drained" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| memory_values="$(awk '/^Mem:/ { print $2, $7; exit }' <<<"${free_bytes_output}")" | ||
| read -r mem_total_bytes mem_available_bytes <<<"${memory_values}" | ||
|
|
||
| if ! [[ "${mem_total_bytes:-}" =~ ^[0-9]+$ ]] || | ||
| ! [[ "${mem_available_bytes:-}" =~ ^[0-9]+$ ]] || | ||
| (( mem_available_bytes > mem_total_bytes )); then | ||
| echo "Could not determine valid total and available memory from 'free -b'; keeping node drained" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if (( node_real_memory_bytes > mem_total_bytes )); then | ||
| echo "Slurm RealMemory ${node_real_memory_bytes} bytes exceeds MemTotal ${mem_total_bytes} bytes; keeping node drained" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| mem_available_gb="$(awk -v bytes="${mem_available_bytes}" 'BEGIN { printf "%.2f", bytes / 1000000000 }')" | ||
| node_real_memory_gb="$(awk -v bytes="${node_real_memory_bytes}" 'BEGIN { printf "%.2f", bytes / 1000000000 }')" | ||
|
|
||
| echo "Memory source: local 'free'" | ||
| echo "Memory comparison: available=${mem_available_gb} GB (${mem_available_bytes} bytes), Slurm RealMemory=${node_real_memory_gb} GB (${node_real_memory_bytes} bytes)" | ||
| echo "Memory snapshot (free -hw):" | ||
| if ! LC_ALL=C free -hw; then | ||
| echo "Could not print the human-readable memory snapshot with 'free -hw'" >&2 | ||
| fi | ||
|
|
||
| if (( mem_available_bytes < node_real_memory_bytes )); then | ||
| echo "Available memory ${mem_available_gb} GB is still below configured memory ${node_real_memory_gb} GB; keeping node drained" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "Drained node leaves enough memory available for Slurm RealMemory; memory recovery confirmed" | ||
| exit 0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| { | ||
| "name": "idle_mem_used", | ||
| "command": "./idle_mem_used.undrain.sh", | ||
| "platforms": ["any"], | ||
| "skip_for_cpu_jobs": false, | ||
| "skip_for_partial_gpu_jobs": false, | ||
| "contexts": ["hc_program"], | ||
| "node_states": ["drain"], | ||
| "on_fail": "none", | ||
| "on_ok": "undrain", | ||
| "reason_base": "[user_problem] $name", | ||
| "reason_append_details": false, | ||
| "run_in_jail": false, | ||
| "log": "slurm_scripts/$worker.$name.undrain.$context.out", | ||
| "need_env": ["CHECKS_NODE_REAL_MEM_BYTES"] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of our checks are already written in Python. Given that this check has Python tests, maybe write it in Python as well, and the testing part would be even easier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python would simplify JSON parsing and enable more direct unit tests. However, most of the checks in this folder currently invoke shell scripts, and these Python tests deliberately exercise the shell command as a black box.
The scripts are mostly set of system commands.
I suggest keeping this check in shell for this PR and reconsidering Python if its logic grows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's a historical mistake that we wrote these checks in Bash. Bash scripts are getting more and more complex, and it's very hard for humans to deal with this, even though they are written by AI agents now.
E.g., in this script, there is JSON parsing and several other parsers and validators; to work with them, you have to be a bash+jq+awk+whatever tools are used. With Python, you only need to be a Python expert.
But it's up to you, we have a lot of bash already :-)