-
Notifications
You must be signed in to change notification settings - Fork 17
Refactor database layer: replace psycopg3 queries with Django ORM asy… #94
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
base: main
Are you sure you want to change the base?
Refactor database layer: replace psycopg3 queries with Django ORM asy… #94
Conversation
|
|
||
| return await cursor.fetchall() | ||
|
|
||
| msgs = await Message.objects.using(self.using).filter(expire__gt=now).all() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will make an exception here and keep the handcrafted query because it's more efficient.
The query uses DELETE...RETURNING which deletes and returns the deleted result in one query.
The new query with django ORM makes two queries (SELECT and DELETE)
| await conn.execute( | ||
| sql.SQL('TRUNCATE TABLE {table}').format(table=sql.Identifier(GROUP_CHANNEL_TABLE)) | ||
| ) | ||
| await Message.objects.using(self.using).adelete() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will also keep the handcrafted query and execute both Truncate calls in one SQL statement.
This should save one roundtrip to the server
|
Just adding my 2c here, we recently re-implemented this library and we went with ORM queries as well in the first place, but they were significantly slower than have a dedication psycopg async connection, as with the ORM implementation you're constantly waiting on the main thread for queries. This was just our experience though, YMMV. |
|
i have not understood! |
|
The Django ORM async interface relies on
This means that the main Django thread (usually started by an asgi server), which is a blocking thread, will be waited on to perform database queries. Hence, you reduce the throughput of channels-postgres because you're limited by the availability of the connection from the main thread. |
|
I assumed it would be slightly slower due to the translation from ORM to SQL but I wasn't aware of I don't see how we can merge it if it causes a big drop in performance. In the meantime, I'll focus on building a benchmark so we have a clearer picture on the performance. |
|
Yeah I'm sorry I don't have a number, it just felt slower. This is definitely a "feeling" assessment, but it makes sense. |
|
Thanks for clarifying — that makes sense. I understand ORM async may reduce throughput. I’ll wait for your benchmark results. hmmmm i have understood something too , thank you so much |
…nc calls