-
-
Notifications
You must be signed in to change notification settings - Fork 22
use_mutation exclusivity #132
-
Consider the following component, which is displayed using {% component ... %} from a template in a browser tab A. On button-click, a lengthy use_mutation is executed.
What I find suspicious is that a copy of the component (or any component, for that matter?) would not load in a parallel tab B until the mutation in A completes. I guess that has to do with component caching.
In fact, I already have trouble accepting that the two mutations don't run in parallel when invoked from tab A and tab B -- I think the exclusivity should be session-scoped, whether work touches Django ORM or not.
from typing import Callable from django.utils.timezone import now from django_idom.hooks import use_mutation from idom import html, use_state, component def work(t0, trg: Callable): import time time.sleep(10) trg(now() - t0) @component def Main(): sleeper = use_mutation(work) elapsed = use_state(None) return html.div( html.button( { 'on_click': (lambda e: sleeper.execute(t0=now(), trg=elapsed.set_value)), 'class_name': "btn btn-primary", 'disabled': sleeper.loading, }, f"Elapsed: {elapsed.value}" if elapsed.value else "use_mutation" ), )
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 4 comments 9 replies
-
the component (or any component, for that matter?) would not load in a parallel tab B until the mutation in A completes.
My suspicion is that this has more to do with the synchronous nature of the task. In short, that time.sleep call is holding up the entire server. @Archmonger, correct me if I'm wrong, but I think the solution here is to run queries and mutations in a separate thread in order to prevent long running tasks from blocking.
Beta Was this translation helpful? Give feedback.
All reactions
-
Hmm, database_sync_to_async (which we use under the hood) already executes queries in a thread...
Beta Was this translation helpful? Give feedback.
All reactions
-
The page loads, but the component does not.
Beta Was this translation helpful? Give feedback.
All reactions
-
@numpde I think the only real solution we have will be to allow you to specify whether your query/mutation is thread sensitive. Unfortunately, as the docs explain, that isn't guaranteed to work.
Beta Was this translation helpful? Give feedback.
All reactions
-
Ok, I've discovered that database_sync_to_async uses a thread pool with only one worker by default. I haven't tested this hypothesis out, but based on that information, I think that all django-idom queries/mutations, while they won't block the main rendering loop, run in serial. This could explain the lack of parallelism you're experiencing since any component which runs a query/mutation would be subject to this bottleneck.
Beta Was this translation helpful? Give feedback.
All reactions
-
Found a similar issue reported here in Django Channels.
Beta Was this translation helpful? Give feedback.
All reactions
-
Unfortunately this has been a long standing issue with how Django handles async.
They've taken the approach of ensuring ASGI causes zero performance impact to WSGI, which unfortunately leaded to horrible async capabilities.
I vaguely recall that Django's thread pool executor can be configured in size, but doing so would break the ORM.
Beta Was this translation helpful? Give feedback.
All reactions
-
At least based on the report in that issue, setting thread_sensitive=False (which effectively increases the thread pool size) didn't seem to break anything for that user. Perhaps there are edge cases where that's an issue though.
Beta Was this translation helpful? Give feedback.
All reactions
-
thread_sensitive was created primarily for sync ORM compatibility. A lot of sync ORMs don't expect hopping threads between middleware and view execution, which was leading to database corruption.
Django fundamentally only has a sync ORM stack. But they fake async using thread sensitive sync_to_async calls. Unfortunately this is used pretty much everywhere in Django safety. At some point they changed the default in asgiref to thread_sensitive=true as a result.
Since Django's async API for the ORM are wrapped with their own sync_to_async decorator, so we shouldn't have a problem disabling thread sensitivity on our mutation and query hooks.
Beta Was this translation helpful? Give feedback.
All reactions
-
Ok, this just seems like something the user should be able to configure if they're confident that their query can be run concurrently then.
Beta Was this translation helpful? Give feedback.
All reactions
-
I suspect the issue here is related to how Django's async functionality is currently faked within Django Core (sync_to_async defaults to thread_sensitive=True).
By default everything async that Django does is single threaded. And IDOM currently relies on Django's async stack.
We might actually be able to get away with disabling that thread sensitive behavior within IDOM though, since IDOM is technically never executing database queries itself.
Beta Was this translation helpful? Give feedback.
All reactions
-
This is now being tracked in the following locations and will be resolved soon
Issue
Pull Request
Beta Was this translation helpful? Give feedback.
All reactions
-
@numpde Can you test out using use_query/use_mutation on my branch #134?
pip install git+https://github.com/Archmonger/django-idom@thread-sensitive
It should resolve the issues you're seeing here in one of two ways:
- You can write your mutation/query as async functions
- Also make sure you use Django's async ORM calls when possible, and
channels.db.database_sync_to_asyncto convert Django's legacy API to async when needed.
- Also make sure you use Django's async ORM calls when possible, and
- You can use traditional sync functions but disable
thread_sensitivebehavior by usingdjango_idom.types.MutationOptionsanddjango_idom.types.QueryOptionsrespectively.- Note: If you disable
thead_sensitiveyou'll need to make sure your middleware isn't touching the same ORM models as you. Under typical configurations, this means your mutations/queries needs to avoid anything related toUserorSessionobjects.
- Note: If you disable
Beta Was this translation helpful? Give feedback.