Capture diagnostics when environment tests stop making progress - #7882
myurasov-nv wants to merge 1 commit into
Conversation
|
There was a problem hiding this comment.
Isaac Lab Review Bot
The journal-backed progress deadline reuses the existing diagnostics, process-group termination, and crash-result reconstruction paths, but two timing details need correction before merge: the progress clock begins during the separately budgeted startup phase, and empty-journal timeout results retain the full file budget instead of the measured elapsed time.
- Design and architecture: Using crash-journal growth rather than console output as the liveness signal is consistent with the intended behavior and preserves a single termination pathway. However, the 1200-second progress window starts at process launch and overlaps the same suites’ 1000-second external-asset startup allowance, leaving only about 200 seconds after an otherwise permitted startup to produce the first journal entry. Start the progress window when startup completes.
- API:
capture_test_output_with_timeoutpreserves its signature and six-element return contract, while documenting the environment-controlled progress deadline. Reusingkill_reason == "timeout"keeps existing downstream reconstruction compatible; no public API break is introduced. - Implementation: Monotonic timing, journal-size polling, diagnostics capture, and process-group cleanup are implemented coherently. Reset
last_progresswhen startup first completes, and pass measuredwall_timeasfallback_time_elapsedso progress-stall failures with an empty journal do not report the much larger overall file budget in synthesized results and aggregate timings.
Minor fixes needed. Posted 2 actionable findings inline.
Automated review; human maintainers own approval decisions.
| shutdown_deadline = 0.0 | ||
| progress_timeout = float(env.get("ISAACLAB_TEST_PROGRESS_TIMEOUT", "0")) | ||
| journal_file = env.get(JOURNAL_ENV_VAR, "") | ||
| last_progress = start_time |
There was a problem hiding this comment.
🟡 Warning · Implementation — Progress deadline overlaps cold git-asset startup window
last_progress is seeded at process launch and only advances when the journal grows, so the 1200 s budget must also cover Kit startup and collection. The same three files declare GIT_ASSET_STARTUP_TIMEOUT (1000 s) in PER_TEST_STARTUP_TIMEOUTS for a cold external asset clone, leaving roughly 200 s for the first journal record before a healthy run is killed and reported as a timeout. Reset last_progress when startup_done first becomes true.
| ovrtx_log_section = ovrtx_log.format_log_section(ovrtx_log.LOG_PATH, pass_file_label) | ||
|
|
||
| msg = f"Timeout after {ctx.timeout} seconds (retried {timeout_attempts} time(s))" | ||
| msg = f"Timeout after {wall_time:.1f} seconds (file budget {ctx.timeout}s; retried {timeout_attempts} time(s))" |
There was a problem hiding this comment.
🔵 Suggestion · Implementation — Synthesized timeout case still records file budget
The log and message now report the measured wall_time, but _make_crash_pass_result is still passed fallback_time_elapsed=ctx.timeout. When a progress stall kills the run at ~1200 s before any journal entry exists, the synthesized TIMEOUT case records the multi-thousand-second file budget instead, inflating shard totals and the slowest-files ranking. Pass wall_time as the fallback duration.
Summary
Add a 20-minute pytest-journal progress deadline for the Isaac Sim PhysX, Newton, and OVPhysX environment suites, with the existing cold-camera allowance. Console noise does not reset the deadline. Collect thread stacks and system diagnostics, kill the subprocess group, and reconstruct completed results through the existing crash journal. Retain each suite’s overall runtime budget.
Motivation
Several task shards reach GitHub’s three-hour job timeout while stuck in environment tests. Their 10,000-second file budgets leave too little time for diagnostics and later test files. In run 35256390526, task shard 2 stopped reporting progress for over 2.5 hours and its hang-dump artifact was empty.
Validation
The underlying simulation hang is not yet attributed; this change captures the evidence needed to diagnose it.