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

Migrate ensure_computing transitions to new WorkerState event mechanism #5895

Closed
3 tasks done
Tracked by #5736 ...
fjetter opened this issue Mar 3, 2022 · 1 comment · Fixed by #6003, #6062, #6092 or #6098
Closed
3 tasks done
Tracked by #5736 ...

Migrate ensure_computing transitions to new WorkerState event mechanism #5895

fjetter opened this issue Mar 3, 2022 · 1 comment · Fixed by #6003, #6062, #6092 or #6098
Assignees

Comments

@fjetter
Copy link
Member

fjetter commented Mar 3, 2022

The task executing transitions should be migrated to the WorkerState event mechanism as outlined in #5736 (comment)

  • The Worker.execute method is modified such that it no longer performs any transition but instead returns appropriate StateMachineEvents that trigger the necessary handlers. For instance
    • TaskFinished
    • Rescheduled
    • TaskErred
  • The ensure_computing method is removed from the list of handle_comm every_cycle callbacks
  • The logic of ensure_computing is moved to a private method (perspectively a method of the new class WorkerState)
  • The new private method will not perform any transitions but rather return a list of recommendations
  • This private method is then called as part of the transition system, e.g.
    • unpause handler
    • transition_waiting_ready
    • transition_executing_*
  • The implicit goal is to remove all invocations of self.loop.add_callback(self.execute, ...) and replace this with the new callback method

Blocked by

Related work

@crusaderky crusaderky reopened this Apr 8, 2022
@crusaderky crusaderky changed the title Migrate ensure_executing transitions to new WorkerState event mechanism Migrate ensure_computing transitions to new WorkerState event mechanism Apr 8, 2022
mrocklin pushed a commit that referenced this issue Apr 13, 2022
@crusaderky crusaderky reopened this Apr 13, 2022
mrocklin pushed a commit that referenced this issue Apr 13, 2022
Upon ``Worker.close()``, clean up the nanny asyncio tasks that were spawned to run ``Worker.execute()``.

- Partially closes_ #5895

## Caveats

- Sync functions will keep running until they either terminate naturally or the process is killed. This PR does *not* change this behaviour.
- This PR removes *one* source of leaks of asyncio tasks. There are many others (chiefly around networking).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment