Skip to content

feat: add monitoring thread to _multi_connect for early timeout exit#4310

Open
germa89 wants to merge 48 commits intomainfrom
feat/early-exiting-timeout
Open

feat: add monitoring thread to _multi_connect for early timeout exit#4310
germa89 wants to merge 48 commits intomainfrom
feat/early-exiting-timeout

Conversation

@germa89
Copy link
Collaborator

@germa89 germa89 commented Nov 18, 2025

Description

This PR adds a background monitoring thread to the _multi_connect method in MapdlGrpc that actively checks if the MAPDL process is alive during connection attempts. This enhancement allows PyMAPDL to exit early if the MAPDL process dies, rather than waiting for the full timeout period.

Key Changes

Modified _multi_connect method in src/ansys/mapdl/core/mapdl_grpc.py:

  • Added a monitoring thread that runs in parallel with connection attempts
  • Thread checks process status using _check_process_is_alive every 0.5 seconds
  • Early exit when process death is detected, reducing wait time from full timeout (~15-30s) to ~1-2 seconds
  • Proper thread cleanup using threading.Event and join() mechanisms
  • Monitoring only activates for local MAPDL instances with available process and path information

Test Coverage:

Added 7 comprehensive unit tests in tests/test_launcher.py:

  1. test_multi_connect_with_valid_process - Tests successful connection with alive process (requires local MAPDL)
  2. test_multi_connect_early_exit_on_process_death - Tests early exit when process dies during connection (requires local MAPDL)
  3. test_multi_connect_monitoring_conditions - Parametrized test for different monitoring start conditions
  4. test_multi_connect_monitoring_thread_cleanup - Verifies thread is properly cleaned up
  5. test_multi_connect_monitor_detects_process_death - Tests monitor detection capability with mocks
  6. test_multi_connect_with_successful_connection_stops_monitoring - Verifies monitor stops after successful connection
  7. test_multi_connect_remote_no_monitoring - Tests that monitoring doesn't start for remote instances

Benefits

  • Faster failure detection: Users no longer wait for full timeout when MAPDL fails to start
  • Better user experience: Immediate feedback when process dies during connection
  • Resource efficiency: No unnecessary waiting when process has already failed
  • Non-intrusive: Only monitors local instances, doesn't affect remote connections
  • Thread-safe: Proper cleanup ensures no resource leaks

Issue linked

This PR addresses the issue where PyMAPDL waits for the full timeout period even when the MAPDL process has already died during connection attempts, particularly when using launch and discovery methods.

Checklist

This commit adds a background monitoring thread to the _multi_connect method that checks if the MAPDL process is alive during connection attempts. This allows PyMAPDL to exit early if the process dies, rather than waiting for the full timeout period.

Key changes: Added monitoring thread in _multi_connect that runs in parallel with connection attempts. Thread checks process status every 0.5 seconds. Early exit when process death is detected. Proper thread cleanup with Event and join mechanisms. Only monitors local MAPDL instances.

Tests added: 7 new tests covering valid connections, early exit on death, monitoring conditions, thread cleanup, process death detection, successful connection cleanup, and remote instance behavior.
@germa89 germa89 requested a review from a team as a code owner November 18, 2025 10:24
@github-actions github-actions bot added the new feature Request or proposal for a new feature label Nov 18, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a background monitoring thread in the _multi_connect method that actively checks if a local MAPDL process is alive during connection attempts. This enables PyMAPDL to exit early if the MAPDL process dies, reducing wait time from a full timeout (~15-30s) to approximately 1-2 seconds. The monitoring thread runs in parallel with connection attempts, checking process status every 0.5 seconds using _check_process_is_alive, and only activates for local MAPDL instances with available process and path information.

Key Changes

  • Added monitoring thread to _multi_connect in src/ansys/mapdl/core/mapdl_grpc.py with proper cleanup using threading.Event and join() mechanisms
  • Implemented 7 comprehensive unit tests covering various scenarios including process death detection, monitoring conditions, thread cleanup, and remote instance behavior
  • Thread monitors process status every 0.5 seconds and triggers early exit when process death is detected

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/ansys/mapdl/core/mapdl_grpc.py Adds monitoring thread to _multi_connect method with proper cleanup and early exit logic when MAPDL process dies during connection
tests/test_launcher.py Adds 7 unit tests validating monitoring thread functionality, early exit behavior, thread cleanup, and proper activation conditions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Add _path check to monitoring thread start condition (requires both process and path)
- Mock _check_process_is_alive in all tests to prevent Mock.poll() issues
- All 8 tests now pass successfully (2 skipped on non-local)
@germa89 germa89 force-pushed the feat/early-exiting-timeout branch from ddc43da to 0cfeba8 Compare November 20, 2025 14:04
@codecov
Copy link

codecov bot commented Nov 20, 2025

Codecov Report

❌ Patch coverage is 87.23404% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.73%. Comparing base (be80897) to head (47d6bad).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4310      +/-   ##
==========================================
- Coverage   91.28%   89.73%   -1.56%     
==========================================
  Files         193      193              
  Lines       15742    15778      +36     
==========================================
- Hits        14370    14158     -212     
- Misses       1372     1620     +248     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added CI/CD Related with CICD, Github Actions, etc examples Publishing PyMAPDL examples documentation Documentation related (improving, adding, etc) labels Jan 5, 2026
@github-actions github-actions bot removed examples Publishing PyMAPDL examples documentation Documentation related (improving, adding, etc) labels Jan 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/CD Related with CICD, Github Actions, etc dependencies maintenance General maintenance of the repo (libraries, cicd, etc) new feature Request or proposal for a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants