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

Revisit event loop blocking functions #1577

Open
2 of 3 tasks
sk1p opened this issue Jan 24, 2024 · 1 comment
Open
2 of 3 tasks

Revisit event loop blocking functions #1577

sk1p opened this issue Jan 24, 2024 · 1 comment

Comments

@sk1p
Copy link
Member

sk1p commented Jan 24, 2024

Remainders from #1572:

  • unsnooze -> get_executor: slow on some machines, so it might need to be async (or can it run in a thread?) Snooze state display in UI #1578
  • shutdown: closing the executor could take a while? (this might need to be refactored somehow, as we can't just make it async... and cleaning up the snooze task can't be done reliably from a different thread, which makes things difficult...)
  • make_executor Snooze state display in UI #1578

Regarding unsnooze and make_executor - both are basically blocked by this issue:

async def create_and_set_executor():
if executor_spec is not None:
# Executor creation is blocking (and slow) but we
# need to run this within the main loop so that Distributed
# attaches to that rather than trying to create its own, see:
# https://github.com/LiberTEM/LiberTEM/pull/1535#pullrequestreview-1699340445
# TL;DR: without this, our call to `loop.run_forever` causes
# `RuntimeError: This event loop is already running`
shared_state.create_and_set_executor(executor_spec)

Also, the cluster spin-up could be more lazy, like only wait for a fraction of workers to come up, instead of all of them, and already start servicing real user requests.

@sk1p sk1p mentioned this issue Feb 22, 2024
13 tasks
sk1p added a commit to matbryan52/LiberTEM that referenced this issue Feb 26, 2024
`snooze`/`unsnooze`/`make_executor`/`get_executor`/`get_context` and
`create_and_set_executor` are now all async, with some blocking
operations running in the background pool.

Ref LiberTEM#1577
@sk1p
Copy link
Member Author

sk1p commented Feb 26, 2024

  • unsnooze -> get_executor: slow on some machines, so it might need to be async (or can it run in a thread?)
  • shutdown: closing the executor could take a while?
  • make_executor

Regarding unsnooze and make_executor - both are basically blocked by this issue:

async def create_and_set_executor():
if executor_spec is not None:
# Executor creation is blocking (and slow) but we
# need to run this within the main loop so that Distributed
# attaches to that rather than trying to create its own, see:
# https://github.com/LiberTEM/LiberTEM/pull/1535#pullrequestreview-1699340445
# TL;DR: without this, our call to `loop.run_forever` causes
# `RuntimeError: This event loop is already running`
shared_state.create_and_set_executor(executor_spec)

This is now taken care of as part of #1578 - shutdown probably can't be async, as that is called too late in the whole process. shutdown is generally a bit broken right now - if it's called from a different thread than where the loop is running, we can't reliably wait for the cancellation to actually go through (and if the event loop is dead, it's pointless anyways)

sk1p added a commit that referenced this issue Feb 26, 2024
`snooze`/`unsnooze`/`make_executor`/`get_executor`/`get_context` and
`create_and_set_executor` are now all async, with some blocking
operations running in the background pool.

Ref #1577
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant