Channels #6419
Channels #6419
Conversation
django/channels/database_layer.py
Outdated
There was a problem hiding this comment.
Maybe a select_for_update() should be added here? As is, multiple consumers are likely to fetch the same message. I'm not sure what guarantees that the same message isn't fetched multiple times.
We could make the db channel implementation better (skip locked in PostgreSQL for example), but it is probably enough to just make it slow but correct.
There was a problem hiding this comment.
My impression was that transaction.atomic() would be enough to provide the isolation here (without it, I was definitely seeing multiple-fetch, and with it, that's never happened).
There was a problem hiding this comment.
Nope, that isn't enough. There is nothing blocking selecting the same message multiple times. On PostgreSQL you can test with begin; select id from test_table order by id limit 1; delete from test_table where id = <fetched_id>; in multiple psql sessions. When the first session commits, the second session unblocks and deletes nothing.
There was a problem hiding this comment.
Hmmh, if you are using REPEATABLE READ or SERIALIZABLE isolation, then the second session is likely going to fail instead of returning nothing. I believe select_for_update is still the right thing to do.
There was a problem hiding this comment.
I tested this, and on PostgreSQL if using REPEATABLE READ with select_for_update() you'll get serialization error on the select instead of delete (which is slightly better). If using READ COMMITTED, then you'll get the next row in the second session which is actually the wanted behavior.
It is possible (though unlikely) that even MySQL behaves sanely with select_for_update().
|
I quickly checked the db layer, and I think it is good enough for development after a couple of changes. I've been wondering if it would be possible to have a bypass of the channel layer for huge responses. Using the channels when the reader of the message is fixed seems like wasted resources. Some ideas are:
|
|
How would you bypass, exactly? As the Django worker, you have no idea where the protocol server is on the network (and if your opsec is done right, you can't talk to it unless you explicitly open up the ports), and the two programs don't share a filesystem or local shared memory necessarily. In my limited testing, the in-memory channel actually outperformed WSGI (mostly due to larger chunking and because it interleaved the processes better). I haven't done any benchmarks of the database layer, as it's not intended for any serious use, and the redis layer I've only done latency benchmarks of (where it tops out at around 150 req/sec with just one Daphne and one Django process) |
django/channels/asgi.py
Outdated
There was a problem hiding this comment.
You can remove this version check.
|
Yes, if the protocol server can't talk to the worker server directly, then there can be no bypass. The interesting case is how will redis perform with large responses. |
|
Yes, this approach will never be as good as WSGI for large responses, but then neither of them are as good as raw C code either. My initial tests showed throughput to be pretty good, but I haven't get tried shoving a few gigabytes down several responses at once and seeing what happens (I suspect Redis might run out of memory and just dump messages depending on how it's configured) |
|
It would be really nice to have some docs about what to do when you need large responses. It might be that a message containing "read response from this URL" approach might actually work. For instance, you could stream large static files that way if the protocol server has access to the static files. Or, you could have a proxy server in between the worker and the protocol server dedicated to streaming data between the servers. |
|
It's certainly possible you could have nginx do the thing where you send a header in the response and it uses that as a path to the file to actually stream. I don't see a lot changing around our general advice though - don't stream huge files from Python, do it from a proxy server in front of gunicorn/uwsgi/daphne/etc. Small static files from things like Whitenoise are fine, we just don't want (and never have wanted) people reading through several hundred megabytes of video when other software/CDNs are much better at that. |
django/channels/routing.py
Outdated
There was a problem hiding this comment.
Another option would be using import_string: https://docs.djangoproject.com/en/1.9/ref/utils/#module-django.utils.module_loading
| ]}, | ||
| install_requires=[ | ||
| 'asgiref>=0.10', | ||
| 'daphne>=0.11', |
There was a problem hiding this comment.
Both of these packages have little to no test coverage. It concerns me that they are going to be core dependencies to Django.
There was a problem hiding this comment.
asgiref has extensive test coverage on the parts we are using (in fact, one of its main jobs is to provide the test confirmance suite). Daphne is less so - see my post to django-developers.
There was a problem hiding this comment.
Yes this discussion is better held on django-developers.
|
|
||
| Default: Value of :setting:`SESSION_ENGINE` | ||
|
|
||
| Allows you to specify an alternative session backend to use for per-channel |
There was a problem hiding this comment.
This should note that django.contrib.sessions.backends.signed_cookies cannot be used. Related that if SESSION_ENGINE is using django.contrib.sessions.backends.signed_cookies then this must be set to use a different engine. Not sure if this is the right place for that note.
| raise CommandError('You must set settings.ALLOWED_HOSTS if DEBUG is False.') | ||
|
|
||
| self.use_ipv6 = options['use_ipv6'] | ||
| self.verbosity = options.get("verbosity", 1) |
There was a problem hiding this comment.
Please cleanup options usage as done in 1845bc1 throughout the new management commands and in the edits.
|
As per my post to django-developers (https://groups.google.com/forum/#!topic/django-developers/QRd4v7OErT8), this patch is withdrawn for now. |
Adds Channels into Django as
django.channels, including tests and documentation ported from the standalonechannelsrepository.