Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat] Asyncio execution policy #3347

Draft
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

Blanca-Fuentes
Copy link
Contributor

@Blanca-Fuentes Blanca-Fuentes commented Dec 13, 2024

This PR changes the asynchronous execution policy to use asyncio (AsyncioExecutionPolicy).

The main changes introduced are the following:

  • KeyBoardInterrupt management: introduction of a new type of exception KeyboardError to raise in case of keyboard interruption during the execution of the code.
  • attach_hooks(): if the hook is the run / wait methods of a test then, they need to be treate as asyncio coroutines
  • Logging context manager: switching the control within the logging context must be treated carefully. I created a _global_logger and a tasks_logger. The _global_logger is used when the asyncio loop has not been yet opened and the tasks_logger to retrieve the right logging context for each task.
  • The stage_dir and the output_dir must be absolute paths for the right context switching with with change_dir_global. The working directory of reframe is now set at the beginning with set_working_dir, it is retrieved at any point in the code with get_working_dir because changing directory with asyncio does not guarantee that we will be in the right directory when doing os.getcwd().
  • The test methods compile, run, compile_wait, run_wait are now asynchronous.
  • New run_command_asyncio method was added to create an asyncio subprocess, which has the same output as the run_command_async. This method waits (asynchronously) for the command to be executed. The method run_command_async_alone just starts the subprocess but does not wait for completion (this allows the local scheduler to poll for the job completion).
  • The termination and killing (_term_all() and _kill_all()) of the processes in the local scheduler is more challenging because the communication of signals between children and parent processes is not handled in the same way. The children of a process are retrieved explicitly and signals are sent to each process individually.
  • _run_strict and _run_strict_s are the asyncio and synchronous versions of _run_strict.
  • The handling of the different cases (tests) takes place inside execute() not in exit() as in the previous AsynchronousExecutionPolicy.
  • Adaptation of the tests for the new asyncio policy.

TODOs

Extend the implementation to other schedulers (only slurm /local)

  • pbs
  • flux
  • sge
  • slurm
  • local
  • ssh
  • oar

Other tasks

  • Test performance vs previous AsynchornousExecutionPolicy
  • Create more tests for the unittests

@pep8speaks
Copy link

pep8speaks commented Dec 13, 2024

Hello @Blanca-Fuentes, Thank you for updating!

Line 355:80: E501 line too long (87 > 79 characters)
Line 367:80: E501 line too long (88 > 79 characters)

Line 97:80: E501 line too long (94 > 79 characters)
Line 313:80: E501 line too long (90 > 79 characters)
Line 321:80: E501 line too long (95 > 79 characters)
Line 435:13: E303 too many blank lines (2)
Line 488:80: E501 line too long (88 > 79 characters)
Line 489:80: E501 line too long (89 > 79 characters)
Line 500:80: E501 line too long (99 > 79 characters)
Line 530:80: E501 line too long (88 > 79 characters)
Line 531:80: E501 line too long (89 > 79 characters)
Line 649:80: E501 line too long (90 > 79 characters)
Line 699:80: E501 line too long (91 > 79 characters)
Line 705:80: E501 line too long (97 > 79 characters)
Line 797:80: E501 line too long (86 > 79 characters)
Line 805:80: E501 line too long (88 > 79 characters)
Line 808:80: E501 line too long (83 > 79 characters)

Line 422:5: E266 too many leading '#' for block comment
Line 426:5: E266 too many leading '#' for block comment
Line 430:5: E266 too many leading '#' for block comment
Line 430:80: E501 line too long (82 > 79 characters)
Line 432:5: E266 too many leading '#' for block comment
Line 432:80: E501 line too long (81 > 79 characters)
Line 438:80: E501 line too long (96 > 79 characters)

Do see the ReFrame Coding Style Guide

Comment last updated at 2025-01-24 12:30:40 UTC

@Blanca-Fuentes Blanca-Fuentes force-pushed the feat/asyncio_exec branch 2 times, most recently from 2a01b52 to c25e50b Compare January 14, 2025 09:01
@Blanca-Fuentes Blanca-Fuentes force-pushed the feat/asyncio_exec branch 4 times, most recently from e35b694 to 02ddcc2 Compare January 15, 2025 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

3 participants