Skip to content

degen_watch.sh: node offense counters hold two lines, so the exclusion test errors out #68

Description

@smjenness

grep -c prints 0 and exits 1 when it matches nothing, so the || echo fallback fires as well and the variable ends up holding two lines.

inst/hpc_doctor/degen_watch.sh:343-344

cpu_offenses=$(grep -cE "^$node cpustarv$" "$STATE_FILE" 2>/dev/null || echo 0)
all_offenses=$(grep -cE "^$node " "$STATE_FILE" 2>/dev/null || echo 1)

When the node has no prior cpustarv event, cpu_offenses becomes 0\n0 and the test on the next line errors:

degen_watch.sh: line 345: [: 0
0: integer expression expected

Repro:

printf 'node1 iostall\n' > sf.txt
x=$(grep -cE "^node1 cpustarv$" sf.txt 2>/dev/null || echo 0)
printf '[%s]\n' "$x"
# [0
# 0]

The same variable is interpolated into the note in the elif branch, which is why that message splits across two log lines:

[doctor]   note: node136 has 4 confirmed events this campaign (0
[doctor]   0 CPU-starvation); not excluding (filesystem stalls are transient/cluster-wide)

Seen 9 times over a two-day India mega-grid campaign on 2.9.2 (9581dd6), across node65, node136, node137 and node138.

Impact looks limited. The failed if falls through to the elif, which was the right branch every time here because all the events were filesystem stalls that are not meant to escalate. The CPU-starvation escalation path is also intact once a node has at least one such event, since grep then exits 0 and the fallback does not fire. So on this evidence it is log noise plus a mangled message rather than a behavior change. It does mean the escalation branch is never reached on the zero-count path, and any future arithmetic on cpu_offenses would hit the same thing.

all_offenses has the same shape. It is safe today only because line 342 appends the node before the grep runs, so that count is never zero.

Suggested fix, keeping a fallback for a missing STATE_FILE:

cpu_offenses=$(grep -cE "^$node cpustarv$" "$STATE_FILE" 2>/dev/null) || cpu_offenses=0
all_offenses=$(grep -cE "^$node " "$STATE_FILE" 2>/dev/null) || all_offenses=1

The script runs under set -u and not set -e, so the || guard is enough.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions