slurm: make job-status retries resilient to real transient failure

[yanok]

Summary

SlurmSystem retries job-status queries (is_job_running,
is_job_completed, get_job_status) on known transient errors — but
the loop re-runs sacct immediately and the error list is fixed. A
single status hiccup can raise out of monitor_jobs and kill a
multi-hour scenario whose job is fine.

Two gaps, one goal:

1. Fast-failing transients exhaust the retry budget in under a
second. Socket timed out self-paces (the client blocks ~10 s per
attempt), but fast failures — e.g. connection refused while
slurmctld/slurmdbd restarts — return in milliseconds, so
retry_threshold=3 only tolerates failures shorter than one
back-to-back sacct invocation. This PR pauses between attempts
(status_retry_pause_seconds, default 10 s, the order of slurm's
message timeout): three attempts now span ~30 s, inside the default
monitor_interval = 60 cadence. The sleep only runs after a failed
attempt.

2. The pattern list can't know site-specific errors. Sites
wrapping the slurm CLIs (proxies, split login-node shims) emit their
own transient transport errors, which today bypass the retry entirely.
extra_transient_status_errors lets the system config extend the
retryable set:

extra_transient_status_errors = ["slurm-proxy:"]

The first commit extracts a shared _is_transient_status_error(): the
check existed in three drifting copies (get_job_status was already
out of sync), and both fixes need one point of truth.

Test Plan

• Unit: tests/systems/slurm/test_system.py — 71 passed; retry tests
  stay instant (fixture pins the pause to 0); new test covers the
  extra-patterns path. ruff check / ruff format clean.
• Production: the equivalent change has run in our internal Slurm
  benchmarking harness since 2026-07-15; multi-hour sweeps now survive
  status hiccups that previously killed them.

Additional Notes

• Three commits, reviewable independently: refactor (no behavior
  change) → pause → config extension.
• Fixed pause, not exponential backoff: with retry_threshold=3 under
  a 60 s monitoring cadence, three evenly spaced attempts over ~30 s
  already cover the observed transient windows.

[coderabbitai]

📝 Walkthrough

Walkthrough

Slurm status queries now support configurable transient-error patterns and retry pauses. Shared detection is used by running, completed, and status queries, with validation and tests for configured patterns and retry behavior.

Changes

Slurm status retry behavior

Layer / File(s)	Summary
Retry policy configuration and detection src/cloudai/systems/slurm/slurm_system.py	Adds configurable transient stderr patterns, validates nonblank entries, configures retry pauses, and centralizes transient-error detection.
Status query retry integration src/cloudai/systems/slurm/slurm_system.py, tests/conftest.py, tests/systems/slurm/test_system.py	Updates all three status-query methods to pause before retries, disables pauses in the fixture, and tests final-retry behavior and configured transient patterns.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: srivatsankrishnan

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: making Slurm job-status retries resilient to transient failures.
Description check	✅ Passed	The description is detailed and directly matches the retry pause, custom transient errors, and test updates in the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 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 `@src/cloudai/systems/slurm/slurm_system.py`:
- Around line 273-279: Update the retry handling in the status-query branches
around _is_transient_status_error and the corresponding branches near the other
retry sites so time.sleep(self.status_retry_pause_seconds) runs only when
retry_count is still below retry_threshold. Apply this guard consistently to all
three retry branches, while preserving logging, counter updates, and immediate
termination after the final attempt.
- Around line 245-246: Update the transient-status matching logic near the
pattern construction in SlurmSystem so blank or whitespace-only entries from
extra_transient_status_errors cannot match every stderr value. Filter invalid
patterns before any(p in stderr for p in patterns), and enforce the same
nonblank constraint at the configuration/model boundary if that validation
mechanism is available.
- Line 114: Constrain the status_retry_pause_seconds configuration field to
non-negative values by adding the appropriate ge=0 validation metadata to its
declaration. Preserve the existing integer type and default value.

🪄 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: ASSERTIVE

Plan: Enterprise

Run ID: 1cc623a7-c98b-4534-8604-eb6f2805f9e0

📥 Commits

Reviewing files that changed from the base of the PR and between 09f728c and b774ced.

📒 Files selected for processing (3)

• src/cloudai/systems/slurm/slurm_system.py
• tests/conftest.py
• tests/systems/slurm/test_system.py

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@src/cloudai/systems/slurm/slurm_system.py`:
- Around line 373-378: Update the transient-error retry flow in get_job_status
so that when _is_transient_status_error(stderr) remains true after
retry_threshold attempts, it raises the underlying failure instead of falling
through to return an empty status list. Preserve the existing retry delay and
continue behavior for attempts that still have retries available, matching the
exhausted-failure handling used by is_job_running and is_job_completed.

In `@tests/systems/slurm/test_system.py`:
- Around line 294-303: Extend test_no_pause_after_final_retry to cover
is_job_completed and get_job_status as well as is_job_running, preferably by
parameterizing the status-query method while preserving the existing retry setup
and expected RuntimeError. Assert each method sleeps exactly twice for
retry_threshold=3, confirming no pause occurs after the terminal attempt.

🪄 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: ASSERTIVE

Plan: Enterprise

Run ID: 65a89542-2bb7-49a6-9455-d41967df7cd6

📥 Commits

Reviewing files that changed from the base of the PR and between b774ced and 732c313.

📒 Files selected for processing (2)

• src/cloudai/systems/slurm/slurm_system.py
• tests/systems/slurm/test_system.py