Skip to content

fix(e2e): isolate download observation from execution timeouts - #1424

Draft
Yue Sun (KayMKM) wants to merge 1 commit into
mainfrom
yuesu/fix-e2e-download-observer
Draft

Yue Sun (KayMKM) wants to merge 1 commit into
mainfrom
yuesu/fix-e2e-download-observer

Conversation

@KayMKM

Copy link
Copy Markdown
Contributor

Summary

  • Move Hugging Face cache traversal and native process-handle inspection into a lightweight disposable observer process instead of the timeout-enforcing interpreter.
  • Consume bounded, nonblocking observations; stale, failed, or missing observers fall back to ordinary execution timing. Preserve owned-download suspension, one-time budget resets when an observed episode ends, and independent download-stall classification.
  • Own CLI and observer process trees with Windows Job Objects assigned before resume, or POSIX process groups. Capture output in temporary files so inherited pipe handles cannot block cleanup.
  • Keep the process-owner API limited to the arguments actually used, remove final-cache-layout validation, and report degraded observation through the existing stderr/result path.
  • Bound disk-full retry to one retry and add regression coverage for observer failures, ownership, timeout accounting, and descendant cleanup.

Investigation

The real-agent diagnostic comparison used the same commit, QNN agent, model order, and external stack sampler:

  • Baseline 157733150: the BERT fill-mask NPU runner crashed with 0xC0000005 while the model-build child was still progressing. The following GPU runner remained inside psutil.open_files() after the model child exited. The run was subsequently canceled after evidence preservation.
  • Observer-off control 157733278: all ten model/device Eval steps passed. This control isolates the observer as the triggering code path; it is not validation of the production implementation in this PR.

The native dump supports a psutil helper-thread/heap-wait failure, but does not establish the precise low-level corruption mechanism. Crash dumps and local investigation artifacts are not included in this PR.

Related: #1400 and #1399.

Validation performed

  • 291 targeted pytest cases passed on Windows, including real disposable process/download tests:
    • tests/unit/eval/test_hf_download_observer.py
    • tests/unit/eval/test_e2e_process_tree.py
    • tests/unit/eval/test_download_observer_integration.py
    • tests/unit/eval/test_run_eval_script.py
  • Ruff and git diff --check passed.
  • An earlier version of this implementation passed the first six steps of the full observer-enabled QNN sequence. BERT cold build was then stopped by the independent memory-safety guard when available Windows commit dropped below 3 GiB. That run was resource-aborted, not a product timeout or crash; two complete hardware repetitions remain unverified. The simplified version has not been re-run through the complete hardware sequence.
  • POSIX ownership has mocked coverage but has not been executed on a POSIX host. The full repository suite was not run.

Draft status / integration required

This branch was developed against 79f1a1b2. Main subsequently merged #1411 (71e04ac7), which independently changes subprocess download monitoring, retained logs/heartbeats, and release evaluation. A merge preflight reports conflicts in:

  • scripts/e2e_eval/run_eval.py
  • tests/unit/eval/test_run_eval_script.py
  • scripts/e2e_eval/README.md

Keep this PR as a draft until the two approaches are reconciled without dropping the new release/logging behavior, the rebased result is retested, and full observer-enabled hardware validation is completed. The 291 passing cases above apply to this branch, not a merge with current main.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant