Repository navigation
Asyncronous daemon ops #7500
Description
The AiiDA GUI package currently includes a REST backend. As part of developing both it and the AiiDA REST API, I am migrating any pure AiiDA endpoints to the AiiDA REST API, starting with the daemon endpoints.
The present implementation in the AiiDA GUI for worker increase/decrease is as follows:
def _wait_for_num_workers(target: int, timeout: float = 5.0, interval: float = 0.1) -> int:
"""Wait for the daemon worker count to reach ``target`` and return the observed value."""
deadline = time.monotonic() + timeout
client = get_daemon_client()
while True:
current = client.get_numprocesses()['numprocesses']
if current == target:
return current
if time.monotonic() >= deadline:
return current
time.sleep(interval)
@write_router.post(
'/increase',
response_model=DaemonStatus,
responses={
500: {'model': errors.DaemonError},
},
)
@with_dbenv()
async def increase_daemon_worker() -> DaemonStatus:
"""Increase the number of daemon workers by one."""
client = get_daemon_client()
if not client.is_daemon_running:
raise DaemonException('The daemon is not running.')
initial = client.get_numprocesses()['numprocesses']
client.increase_workers(1)
num_workers = _wait_for_num_workers(initial + 1)
return DaemonStatus(running=True, num_workers=num_workers)The ad-hoc _wait_for_num_workers is unfortunate. Perhaps increase/decrease (and maybe other ops?) could be made async, to be awaited.
Thoughts? @agoscinski @GeigerJ2 @khsrali
Pinging also @Bud-Macaulay, who inquired about this during PR review.
Activity
khsrali commentedon Jul 29, 2026
Thanks for the ping @edan-bainglass , a few things:
If you are asking to make
clientan awaitable object, then that's a huge change which perhaps requires dropping cricus, and switch to something asynchronous. @agoscinski can comment on that.And just a side note for Alex: if you ever plan to implement this, one has to be careful how to handle the event loop. In this case runner should bind uvicorn's not the other way around, as aiida-core should always own the loop.
I'd personally vote against implementing this, because it imposes a large chunk of changes for our future handling logic in aiida-core.
That function is written I'm afraid, not very well. First of all nothing stops it from being async. And second, you should at least use
await asincio.sleepin that while loop, instead of overwhelmingly call on client.That would already yield your event handler and reduces the synchronous load on your restapi server.
edan-bainglass commentedon Jul 30, 2026
Yes, I pinged @agoscinski because this is the plan, as far as I'm aware 🙂
And just a side note for Alex: if you ever plan to implement this, one has to be careful how to handle the event loop. In this case runner should bind uvicorn's not the other way around, as aiida-core should always own the loop. I'd personally vote against implementing this, because it imposes a large chunk of changes for our future handling logic in aiida-core.
Against dropping circus, or against making this machinery asynchronous? In any case, can you say more? 🙏
Thanks @khsrali. I will test this out in the REST API. Cheers.
Bud-Macaulay commentedon Jul 30, 2026
I agree that there is no reason why the function can't be async, and i had a very similar comment to Edan.
However, i think we have to write a code block like this just to set the number of daemons to a specific number is a smell of something missing in the daemon client or perhaps in circus itself, (which since Alex is migrating is good to discuss)...
I would prefer if we shovel this async/await as far upstream as we can (into aiida core). Otherwise you mandate that all consumers of the aiida daemon (REST, CLI, python-api, etc) have to implement this await pattern themselves - which i'm sure they'll likely diverge.
Bud-Macaulay commentedon Jul 30, 2026
Actually, i've just looked at the DaemonClient class itself and perhaps much of the issues could be solved by leaning on it a bit more.
I'll comment my suggestions in the AiiDA-RESTAPI (PR 166).
edan-bainglass commentedon Jul 30, 2026
@agoscinski can we get you input on this? 🙏
khsrali commentedon Jul 30, 2026
Against turning DaemonClient's existing methods into coroutines. Which is a public API, and it's called from places that have no event loop and shouldn't have one -- the verdi cli, tests, plugin code. If
get_numprocesses()becomes awaitable, every one of those callers needs either anasyncio.runwrapper or a loop it now has to own.Or we have to provide both sync and async methods as we did for transport, which I don't think if it worth it in this case.
The other thing is that the loop in daemon process itself belongs to the Runner, so we'd be adding a second place where loop ownership has to be debate about. That's a lot of headache and a breaking change for downstream, in exchange for very little gain in REST endpoint.
khsrali commentedon Jul 30, 2026
The current implementation of DaemonClient's clients is meant to be sync.
We could add an optional flag that it would only return when the increased had happened (currently return when the message is acknowledged), something like:
default value for
waitingwould be False for backward compatibility.@edan-bainglass @Bud-Macaulay, let me know if this solves it. It's a one line implementation on our side.
edan-bainglass commentedon Jul 30, 2026
Thanks @khsrali. Indeed, this is an effective shovel 👍 @Bud-Macaulay correct me if I'm wrong.
I only suggest
waitinstead ofwaiting🙂Bud-Macaulay commentedon Jul 30, 2026
To quote reply thoughts from the restapi thread that Edan has just closed haha.
I think this is only true because we are returning on message receive through circus not message success right?
I think the oneline change
(wait=True)tells the function to wait for the message to succeed, not just be acknowledged. I think perhaps it would be nice to know why this is not the default behaviour for the Daemon Client/message passing.Perhaps naively, if we were to make this the default behavior of the daemon client? Then the methods can be async and the promises can be handled downstream accordingly (i.e.failure/success)?
Bud-Macaulay commentedon Jul 30, 2026
But perhaps this is more of a discussion point/design consideration for the work alex is looking at, (which im sure hes thought about much more than I ever will)
khsrali commentedon Jul 30, 2026
very good questions
Yes and no. Daemon lives in a separate process and you'll have to reach it, no reaching is immediate, unless you wait.
There's no other tool on the market that can do this for you immediately.
And if you send conflicting messages (incr, decr) one after the other -if you don't wait for acknowledgment-, there's no guarantee that they execute in order that you send them, as far as i understand.
That's a good question. short answer IDK. Stupid answer: perhaps we care more about the messages being acknowledged than being executed since we have persistency, and to speed up.
Theoretically yes, but like i said making the DaemonClient async, is a large work and there should be a large gain to do that, which I fail to see. And we have cli which are entirely sync functions, so you'll have to provide also a sync counterpart which inflates amount of code.