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 20 commits into
base: develop
Choose a base branch
from

Conversation

Blanca-Fuentes
Copy link
Contributor

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.

TODO:

  • Extend the implementation to other schedulers (only slurm /local)
  • Test performance vs previous AsynchornousExecutionPolicy
  • Create more tests for the unittests

@pep8speaks
Copy link

Hello @Blanca-Fuentes, Thank you for submitting the Pull Request!

Line 113:1: E302 expected 2 blank lines, found 1

Line 648:80: E501 line too long (80 > 79 characters)
Line 665:80: E501 line too long (82 > 79 characters)

Line 374:1: E302 expected 2 blank lines, found 1

Do see the ReFrame Coding Style Guide

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.

2 participants