This repository has been archived by the owner on Jun 29, 2020. It is now read-only.
Acitivity 激重に対する SQL レベルでの修正 #945
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
django_activity のスロークエリが二回飛んでいた問題を修正
{% if activities|length > 10 %} の部分が下記のような流れになるらしい
1. SELECT * FROM ... の SQL を投げ Python のリスト化
2. Python のリストに対して len() を呼ぶ
そのため上記部分を {% if activities.count > 10 %} にすることで SELECT * ではなく
SELECT COUNT(*) が呼ばれるように修正した
django_activity の場合サブクエリにて content_object 毎に最新の activity を取得するという処理を行っている。
したがってサブクエリに対しての LIMIT/OFFSET 適用が可能であり、そちらのほうがリクエストするオブジェクト数が少なくなる気がする（未確認）
変更前
SELECT ... FROM activities_activity WHERE id IN (
SELECT MAX(id) AS pk FROM activities_activity GROUP BY ... ORDER BY NULL DESC
) ORDER BY created_at DESC LIMIT 10 OFFSET 10
変更後
SELECT ... FROM activities_activity WHERE id IN (
SELECT MAX(id) AS pk FROM activities_activity GROUP BY ... ORDER BY created_at DESC LIMIT 10 OFFSET 10
) ORDER BY created_at DESC
| @@ -49,7 +49,7 @@ <h4 class="modal-title">ようこそ、Kawazへ！</h4> | |||
| </div> | |||
| {% get_latest_activities as activities %} | |||
| {% include "activities/components/activity_container.html" with activities=activities|slice:":10" %} | |||
| {% if activities|length > 10 %} | |||
There was a problem hiding this comment.
俺もそう思ってた。
本番鯖でSQL見た所問題のクエリが2回投げられてて、なんでかなーって見てたところ、ここで呼んでた。詳細はコミットログ
| 'SELECT *, MAX(id) AS id FROM activities_activity GROUP BY ' | ||
| 'content_type_id, object_id ORDER BY id DESC' | ||
| ) | ||
| qs.count = lambda: len(list(qs)) |
There was a problem hiding this comment.
うっ、厳しそう
ORM使えるなら使いたいけど仕方なさそうですな
There was a problem hiding this comment.
https://github.com/django/django/blob/master/django/db/models/sql/query.py#L157 のコメント的には下記コードで動くべきだと思うんだが
qs = super().get_queryset()
qs.query.group_by = ['content_type_id', 'object_id']
qs = qs.annotate(max_id=Max('id'))
qs = qs.order_by('max_id')これで出てきて欲しいのは上記で書いたような SQL なんだけど、実際は
SELECT *, MAX(id) AS max_id FROM activities_activity GROUP BY content_type_id, object_id, id, _snapshot, ... ORDER BY id DESC
と参照してるフィールド全部羅列される。これは https://github.com/django/django/blob/master/django/db/models/sql/query.py#L1677 のせいっぽい。
もうすこし詳しく調べてみるけど、とりあえずこのブランチで速度上がるかテストして欲しいって感じ
There was a problem hiding this comment.
group_byは複雑かつ罠が多いからdjangoが公式にサポートすることは無いと思うよ
|
通しますわよ |
giginet
added a commit
that referenced
this pull request
Aug 28, 2015
…vity Acitivity 激重に対する SQL レベルでの修正
giginet
added a commit
that referenced
this pull request
Sep 1, 2015
Revert "Merge pull request #945 from kawazrepos/fix/slow_query_in_dja…
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#867 #944
この修正しても遅いとは思うけど、とりあえず SQL 側での対応をしてみた。
この変更でどれくらい早くなる（or 変わらない・遅くなる）のか調査してほしい @miio