Harden daemon lifecycle and clarify reports

[GHoon-Lee]

Summary:

• Verify stale PID files point to the managed gua daemon before status/stop acts on them.
• Clarify report output: sample basis, classification rules, interval-dependent GPU-hours, and heatmap density.
• Split §2 into idle-held capacity vs truly-idle capacity and aggregate §4 identities by identity/GPU/tick.
• Warn when NVML process-list visibility is unavailable and refresh bare-metal 1.0 status docs for 1.0.1.

Validation:

• uv run ruff format --check
• uv run ruff check
• uv run mypy
• uv run pytest
• uv build --out-dir /tmp/gua-dist-review-cleanup
• bash scripts/smoke-dist-wheel.sh /tmp/gua-dist-review-cleanup/gpu_usage_audit-1.0.1-py3-none-any.whl

[GHoon-Lee]

리뷰 요약

전반적으로 1.0.1 이후 cleanup으로 매우 잘 다듬어진 PR입니다. PID 안전성, report wording, §2/§4 집계 정합성, NVML 경고 모두 실제 운영에서 부딪힐 만한 문제를 정확히 짚었고 테스트 커버리지도 함께 들어갔습니다. blocking 이슈는 없고, 아래는 머지 전 또는 follow-up으로 다듬으면 좋을 항목들입니다.

---

SWE 관점

1. §2 equiv_gpus 분모가 조용히 바뀜 (행동 변화)

   src/gpu_usage_audit/report.py:106-108 — 이전 WASTE_QUERY는 (SELECT COUNT(DISTINCT gpu_uuid) FROM gpu_sample)로 DB 전체 카드 수를 분모로 썼는데, 새 gpu_count CTE는 FROM s라서 window 내 활성 카드 수만 셉니다. 이건 개선이지만 (긴 --since 후 짧은 window를 보면 equiv가 과대계상되던 버그가 사라짐), 같은 DB로 1.0.1 vs 새 binary를 돌리면 §2 숫자가 달라집니다. CHANGELOG에 "equiv_gpus는 이제 window 내 카드 수 기준"이라고 한 줄 추가 권장.

2. §4 gpu_hours도 행동 변화

   TOP_IDENTITIES_QUERY에서 owned CTE로 (identity, gpu, ts) 단위 접기를 하면서 같은 사용자가 같은 GPU/tick에 띄운 N개 프로세스가 1로 줄어듭니다. 이전 버전 대비 사용자별 gpu-hours가 줄어드는 케이스가 있다는 점은 CHANGELOG의 "aggregate by identity/GPU/tick"이 암시하지만, 기존 보고서와 숫자가 달라진다는 운영자 영향까지는 명시되어 있지 않아 한 줄 더 풀어주면 좋습니다.

3. _pid_is_managed_daemon의 spawn 명령 강결합

   src/gpu_usage_audit/__main__.py:689-694가 -m gpu_usage_audit daemon 시퀀스를 정확히 찾는데, 이건 _cmd_gua_start의 spawn([sys.executable, "-m", "gpu_usage_audit", "daemon", ...])과만 일치합니다. 향후 누군가 spawn 방식을 gua daemon --foreground console_script 호출로 바꾸면 status/stop이 조용히 "not a gua daemon"으로 떨어집니다. 두 함수 위에 짧은 docstring으로 반드시 같이 변경해야 한다 표시 권장. (혹은 spawn 시 marker env var를 자식에 넣고 /proc/PID/environ에서 확인하는 방식으로 decouple하는 방법도 있지만 1.0 범위 밖이라 메모만.)

4. _pid_is_managed_daemon / _read_proc_cmdline 직접 단위 테스트 부재

   tests/test_smoke.py:334,355가 함수를 monkeypatch해서 호출자 통합만 검증합니다. argv 파싱 자체 (-mfoo glued form, -m이 마지막 arg, 빈 cmdline, 비-Linux fallback empty list 등)에 대한 단위 테스트는 없습니다. 한두 개 추가 권장.

5. _pid_is_managed_daemon 루프 조기 종료

   __main__.py:689-694에서 매칭 후 return True라 사실상 break지만, -m이 여러 번 등장하는 비정상 cmdline에서 두 번째 매치까지 도는 것은 의미가 없습니다. 가독성 차원에서 코멘트 한 줄로 의도 표기.

---

SRE 관점

1. gua status가 side-effect를 가짐

   _cmd_gua_status가 not a gua daemon이거나 stale 일 때 pid file을 지웁니다 (__main__.py:432). status 류 명령은 보통 read-only가 관례라 운영자가 두 번 호출했을 때 두 번째 결과가 첫 번째와 다를 수 있어 모니터링/스크립트에서 헷갈릴 여지가 있습니다. 의도된 설계라면 README의 status/stop 절에 "status는 stale pid file을 정리할 수 있다"를 한 줄 명시하는 게 안전합니다.

2. TOCTOU on kill

   _pid_alive + _pid_is_managed_daemon 통과 후 os.kill(pid, SIGTERM) 사이에 daemon이 종료하고 동일 PID가 재사용될 가능성은 여전히 남습니다. 다만 (a) 윈도우가 매우 작고, (b) 같은 user uid 프로세스에만 영향이 가므로 (c) 1.0.1에서 잡은 stale-PID-file 케이스보다 위험이 훨씬 작습니다. 1.0 범위에서 추가 락은 과합니다. 한 줄 의도 코멘트만 추가 권장.

3. NVML warning rate-limit이 daemon process lifetime 한정

   nvml.py:62-65의 _process_list_warning_uuids는 collector 인스턴스 수명만 유지됩니다. daemon이 재시작되면 다시 warning이 한 번씩 찍힙니다. 단일 호스트 카드 수가 bounded이므로 로그 폭증 위험은 없고 이 동작이 오히려 재시작 후에도 운영자가 경고를 한 번은 볼 수 있어 SRE 입장에서 적절합니다. 변경 불필요, 이대로 OK.

4. 로깅 경로 검증

   _configure_logging이 stderr로 가고, background daemon은 stdout/stderr를 모두 log_path로 redirect 합니다 (__main__.py:388-396). 따라서 새 NVML warning은 ~/.cache/gua/gua.log 같은 곳으로 들어갑니다. 동작 OK, 추가 액션 없음.

---

보안 관점

1. PID kill 권한 모델은 안전

   os.kill은 동일 uid에 한정되어 root daemon을 일반 user가 죽일 수 없습니다. PID 재사용 케이스에서 다른 user의 프로세스를 건드릴 일은 없습니다.

2. /proc/PID/cmdline 읽기는 무해

   읽기 권한이 없으면 OSError로 빈 리스트 반환 → False로 떨어져 보수적으로 "managed가 아니다"로 판단합니다. 디코딩도 errors="replace" 처리되어 binary garbage에 robust.

3. 새 SQL 인젝션 표면 없음

   모든 신규 쿼리는 parameterized. ? placeholder만 사용. OK.

4. 로깅에서의 정보 노출

   NVML warning에 UUID와 NVML error만 들어가고 PID/loginuid 같은 민감 정보는 없음. OK.

---

확장성 관점

1. SQL 비용

   s CTE는 (gpu, tick) 단위 한 번의 hash aggregate이고, headline이 이미 동일 패턴을 쓰고 있어서 추가 비용은 미미합니다. gpu_count 서브쿼리는 distinct 1회. 1d × 60 카드 × 1s ticks ≈ 5M rows 까지는 SQLite에서 문제 없을 것으로 보입니다. v1 schema가 gpu_sample(ts) 인덱스가 있어야 cutoff 필터가 빠른데, 그 부분은 PR 범위 밖이라 별도 확인 사항.

2. §4 LIMIT 10

   identity가 수백~수천 단위로 늘어나도 LIMIT으로 잘리지만, GROUP BY가 전체를 다 돌고 ORDER BY DESC LIMIT 10이라 풀스캔 비용은 그대로입니다. 1.0 범위에선 OK.

---

유저 관점

1. report 헤더 추가 (basis/rules/legend) — 매우 좋음

   §1의 basis와 rules, §2의 --interval 의존성, §3 부제, §4 "identity counts once per GPU/tick", §5 darker-means-active 안내까지 — 보고서를 처음 보는 운영자가 "이 숫자가 뭔지" 묻기 전에 답이 보입니다. 이 변경이 사용자에게 가장 큰 가치.

2. §4 samples 컬럼 추가

   gpu-hours만 보면 "이 사람이 큰 카드 하나만 잠깐 잡았나, 작은 카드 여러 개를 길게 잡았나" 구분이 안 됐는데 samples 컬럼으로 해소. 좋은 추가.

3. "not a gua daemon" 메시지 톤

   gua daemon: removing stale pid 1234 (not a gua daemon) — 짧고 정확하지만 처음 보는 운영자는 "내 daemon이 아니라는 게 무슨 뜻이지? 누가 침투한 건가?"로 잠깐 헷갈릴 수 있습니다. 다음처럼 살짝 더 풀어주는 안도 고려:

   gua daemon: pid 1234 belongs to another process; clearing stale pid file

   사소한 wording 제안.

4. rules 라인 폭

   rules: active >=10% util; idle-held <10% util with >100 MB process memory — 약 71자라 좁은 tmux split (60자)에서 wrap됩니다. 1.0 범위에선 신경 안 써도 되지만, 후속에서 줄바꿈 또는 보다 짧은 표현(예: rules: active≥10% util · idle-held: util<10%, mem>100MB) 고려.

5. gua status의 side-effect 노출

   위 SRE #1과 동일 — README의 status 설명에 "stale pid file을 정리한다"를 한 줄 명시 권장.

---

머지 차단성 없음

남은 모든 항목은 wording/문서/follow-up 수준입니다. 1.0.2 RC 후보로 그대로 진행 가능.

[GHoon-Lee]

리뷰 코멘트 반영했습니다.

• CHANGELOG에 §2 equivalent-GPU 기준이 report window 내 GPU 기준으로 바뀌는 점을 명시했습니다.
• CHANGELOG에 §4가 identity/GPU/tick 단위로 접히면서 기존 대비 per-user GPU-hours가 낮아질 수 있음을 명시했습니다.
• _pid_is_managed_daemon docstring에 _cmd_gua_start spawn shape와 함께 바뀌어야 한다는 계약을 추가했습니다.
• _pid_is_managed_daemon / _read_proc_cmdline 테스트를 추가했습니다. 현재 pytest는 124 passed입니다.
• gua status가 stale PID file을 정리할 수 있음을 README에 명시했습니다.
• stale PID가 live-but-unrelated process일 때 메시지를 belongs to another process; clearing/cleared stale pid file로 풀었습니다.
• stop 경로의 check-then-kill TOCTOU 한계는 코드 코멘트로 남겼습니다.

검증:

• uv run ruff format --check
• uv run ruff check
• uv run mypy
• uv run pytest -> 124 passed

PR CI도 다시 통과했습니다.

[GHoon-Lee]

Follow-up 리뷰 — 46cafdc

이전 리뷰의 actionable 항목이 모두 깔끔하게 반영됐습니다. 각 항목 확인:

반영 확인

• CHANGELOG §2 / §4 행동 변화 명시 ✓ — "GPUs present in the report window instead of the entire database", "reports may show lower per-user GPU-hours when one user has multiple processes on the same GPU at the same tick". 운영자가 이전 보고서와 숫자를 비교할 때 헷갈리지 않을 만큼 정확합니다.
• _pid_is_managed_daemon 계약 docstring ✓ — "Keep this in sync with the spawn command in _cmd_gua_start"로 두 함수 결합을 명시. 향후 spawn 변경 시 누가 봐도 두 군데를 동시에 손대게 됩니다.
• _pid_is_managed_daemon / _read_proc_cmdline 단위 테스트 ✓ — parametrize 6 케이스가 정확히 짚어야 할 엣지(다른 sub-command, 다른 module, console-script 형태, -m이 마지막 arg, 빈 argv)를 다 커버합니다. test_read_proc_cmdline_returns_empty_for_missing_pid도 실제 /proc 부재 fallback을 확인하고 cross-platform에서도 안전합니다.
• README §status 부수효과 명시 ✓ — 표와 daemon 절 둘 다 "stale PID file을 정리한다"를 노출.
• 메시지 wording ✓ — belongs to another process; clearing/cleared stale pid file로 풀어쓰면서 의미가 명확해졌습니다.
• stop의 TOCTOU 한계 ✓ — _cmd_gua_stop에 3줄 코멘트로 잔존 race를 인정. 1.0 범위에서 합리적인 처리.
• pytest 124 passed ✓ — green.

한 가지 남은 nit (non-blocking)

_cmd_gua_status 메시지의 동사 시제가 출력 순서와 미세하게 어긋납니다:

if _pid_alive(pid):
    print(... "cleared stale pid file)")   # 과거형
...
_unlink_if_exists(pid_path)                  # 출력 *후* 실제 unlink

비교:

• _cmd_gua_start: print("clearing ...") → unlink (현재진행, 순서 일치)
• _cmd_gua_stop: unlink → print("cleared ...") (과거, 순서 일치)
• _cmd_gua_status: print("cleared ...") → unlink (과거인데 순서 역)

사용자 체감으로는 동시에 일어나므로 동작상 문제는 없습니다. _cmd_gua_status만 print 전후로 unlink를 옮기거나 "clearing"으로 통일하면 깔끔해질 정도의 사소한 점입니다. follow-up이나 1.0.2 cleanup에 묶어도 충분합니다.

결론

머지 가능합니다. 남은 nit은 cosmetic이고 1.0 범위 안에서 별도 처리 없이도 무방합니다.

[GHoon-Lee]

Follow-up nit도 반영했습니다.

• _cmd_gua_status의 live-but-unrelated PID 경로에서 pid file을 먼저 unlink한 뒤 cleared stale pid file 메시지를 출력하도록 순서를 맞췄습니다.

추가 검증:

• uv run ruff format --check
• uv run ruff check
• uv run mypy
• uv run pytest tests/test_smoke.py -> 44 passed

PR CI도 다시 통과했습니다.