Skip to content
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

DBのクエリが多すぎる #6813

Closed
SanMurakami opened this issue Nov 7, 2020 · 47 comments
Closed

DBのクエリが多すぎる #6813

SanMurakami opened this issue Nov 7, 2020 · 47 comments
Assignees
Labels
packages/backend Server side specific issue/PR 🐢Performance Efficiency related issue/PR

Comments

@SanMurakami
Copy link
Contributor

💡 Summary

毎秒800〜1500ぐらいのクエリがPostgreSQLに飛んでいて、PostgreSQLとMisskeyを違うネットワークに置くと極端に重くなる。

参考GIF(内容が見えないようにリサイズしてます)
dJQkjHx4qG

🙂 Expected Behavior

DB内部である程度処理してから結果を返す

☹️ Actual Behavior

全部Misskey側で処理している

📝 Steps to Reproduce

  1. PostgreSQLのログを見れるようにする
  2. ユーザーが多いインスタンスを作る

📌 Environment

Misskey.io v12.54.0

@SanMurakami SanMurakami added the ⚠️bug? This might be a bug label Nov 7, 2020
@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

全部Misskey側で処理している の詳細が知りたい

@syuilo syuilo added the packages/backend Server side specific issue/PR label Nov 7, 2020
@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

例えばタイムラインを取得するクエリを例にすると、取得されることになる各ノートのユーザー情報を予めJOINするようにしているからDBで出来る限りのことはやっている状態ではなかろうか
https://github.com/syuilo/misskey/blob/30c9c3739f086a7f9df2c4d8ccae78c605a25bb9/src/server/api/endpoints/notes/timeline.ts#L125

@rinsuki
Copy link
Contributor

rinsuki commented Nov 7, 2020

packManyがpackを全部並列で呼び出してるのでfindOneが呼ばれまくってると思う
本来はpackManyでよしなにやってpackがpackManyのラッパになっているべき

別プロジェクトだけど例 https://github.com/rinsuki/sea/blob/5766344c7463dd856ea27d83a47245d764b14cac/src/db/repositories/post.ts (これも application は手抜いてfindOneを投稿数ぶん呼んでるのでよくない)

@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

packMany はそれぞれに pack を呼び出していて、pack は渡されたものがオブジェクトなら find せずにそのまま利用しているから新たなクエリは発生しないように見える
https://github.com/syuilo/misskey/blob/30c9c3739f086a7f9df2c4d8ccae78c605a25bb9/src/models/repositories/note.ts#L95

@rinsuki
Copy link
Contributor

rinsuki commented Nov 7, 2020

noteのrepositoryだとpopulateEmojisとかだめそう

@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

populateEmojisはカスタム絵文字が含まれてない限りクエリ発火しなさそうだけど populateMyReaction は毎回発火してそう

@mei23
Copy link
Contributor

mei23 commented Nov 7, 2020

大きいインスタンスでクエリ数稼いでるのがたぶんこの辺
https://github.com/mei23/misskey-v12/pull/686/files
↑はsaveで無駄にトランザクションとSELECT流してるところを修正しているけど
この辺をクエリ的に減らすのは難しいかも

@rinsuki
Copy link
Contributor

rinsuki commented Nov 7, 2020

populateEmojisはカスタム絵文字が含まれてない限りクエリ発火しなさそう

それはそうなんだけどカスタム絵文字がめっちゃ付いてるとだいたいおもしろいのでRNされまくって刺さり度上がりそう (まあコーナーケースではある)

@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

大きいインスタンスでクエリ数稼いでるのがたぶんこの辺

ノート作成はいろいろやることあるからクエリ多いだろうなぁ

@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

write時のクエリを減らすのはめいめいが言っているように厳しそうだけど、read時のクエリはなんか工夫すれば減らせそうな雰囲気はするな

@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

populateEmojis は見る感じやってることが複雑だからDBサイドでやるのは難しそう

@rinsuki
Copy link
Contributor

rinsuki commented Nov 7, 2020

packManyの時notesの分集約して一回のクエリで問い合わせればマシになりそう

@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

あと気になっているのはデータベースという割と頻繁にアクセスされるであろうシステムを地理的に離れたサーバーで動かすのは割と良くあることなんだろうか

@rinsuki
Copy link
Contributor

rinsuki commented Nov 7, 2020

DB分けないとアプリサーバーの数増やせなくない?

@mei23
Copy link
Contributor

mei23 commented Nov 7, 2020

pack系は非アクティブユーザーの分は処理しないからさほど増大しないけど
Note作成後にミュートやアンテナ見る所は休眠ユーザー分も処理するから登録ユーザーが増えるに従って増大していきそう

@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

DB分けないとアプリサーバーの数増やせなくない?

サーバー自体を分けるのはあると思うけど、それを地理的に離れている場合でもやるのかなって思った

@SanMurakami
Copy link
Contributor Author

離れてると言っても国内だから20msぐらいの遅延だし、地理的に離れてないと以前の震災みたいに影響を受ける

@rinsuki
Copy link
Contributor

rinsuki commented Nov 7, 2020

サーバー自体を分けるのはあると思うけど、それを地理的に離れている場合でもやるのかなって思った

2リージョン以上で同じアプリ動かす時には素直にやるとDBをどっちかのリージョンに寄せないといけない気がするけどあんまり詳しくない (まあリードレプリカとかでマシにはなるだろうけどそれでもwriteはプライマリに行くはずだし)

@SanMurakami
Copy link
Contributor Author

SanMurakami commented Nov 7, 2020

2リージョン以上で同じアプリ動かす時には素直にやるとDBをどっちかのリージョンに寄せないといけない気がする

元々はPgpool-IIでロードバランシングしてたけど、クエリが多すぎるせいかPgpool-II噛ませるだけでかなりパフォーマンスが下がる
今回の構成も最後の方にPgpool-II入れてPostgreSQLでストリーミングレプリケーションしている同一ネットワーク内のDBにウェイトを置いてたけどそれでもめちゃめちゃ重かった

@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

Note作成後にミュートやアンテナ見る所は休眠ユーザー分も処理するから登録ユーザーが増えるに従って増大していきそう

一定期間使用がなかったアカウントのミュートとかアンテナは削除するか一時停止みたいな処理しないとダメかなぁ
まあMisskeyくらいの規模ならデータベースに負荷がかかるほどのミュート/アンテナ設定済み休眠アカウントはなかなか発生しない気はする

@syuilo
Copy link
Member

syuilo commented Nov 7, 2020

負荷を考える上では休眠かどうかは考慮しなくて良いか

@syuilo syuilo added 🐢Performance Efficiency related issue/PR and removed ⚠️bug? This might be a bug labels Nov 7, 2020
@syuilo
Copy link
Member

syuilo commented Nov 8, 2020

需要の無さそうなautoWatch機能を削除したのでノート作成時のクエリが若干減った
syuilo/Misskey.old@9d1fa3f

@syuilo
Copy link
Member

syuilo commented Nov 8, 2020

(需要があったとしてもサーバーサイドでやる必要はない)

@syuilo
Copy link
Member

syuilo commented Mar 18, 2021

syuilo/Misskey.old@4f24915 とか syuilo/Misskey.old@8aa0891 あたりでクエリ数まあまあ減った

@syuilo
Copy link
Member

syuilo commented Mar 18, 2021

packManyの時notesの分集約して一回のクエリで問い合わせればマシになりそう

これやるか

@syuilo syuilo pinned this issue Mar 18, 2021
@syuilo
Copy link
Member

syuilo commented Mar 20, 2021

populateEmojis もまとめて行うようにするか

syuilo added a commit that referenced this issue Mar 20, 2021
@syuilo
Copy link
Member

syuilo commented Mar 20, 2021

やった

@syuilo
Copy link
Member

syuilo commented Mar 20, 2021

ここまでの諸々の改修の結果

タイムラインを100ノート分、通知を100個分読み込んだ時の総発行クエリ

1クエリで3msのオーバーヘッドがあると仮定した場合の時間も併記する

before

797 (2391ms)

  • notifications: 542クエリ
  • timeline: 255クエリ

after

25 (75ms)

  • notifications: 11クエリ
  • timeline: 14クエリ

@mei23
Copy link
Contributor

mei23 commented Mar 20, 2021

フォロワー検索の場合
src/server/api/endpoints/users/followers.ts

	.innerJoinAndSelect('following.follower', 'follower')

フォローされてる検索の場合
src/server/api/endpoints/users/following.ts

	.innerJoinAndSelect('following.followee', 'followee')

あたりを表結合するといいはず

@mei23
Copy link
Contributor

mei23 commented Mar 20, 2021

あと、API (TL/show) 系でクエリされる場合、必ず (デフォルトのDetailedPackedNoeでは) reply, renote, app は解決されるので

.leftJoinAndSelect('note.reply', 'reply')
.leftJoinAndSelect('note.renote', 'renote')
.leftJoinAndSelect('note.app', 'app')

あたりを最初から結合しとくといいかも。
どうせID=NULLならどれともマッチしないのでJOIN負荷は変わらない(と思う)

→ app以外は既に入ってた

@mei23
Copy link
Contributor

mei23 commented Mar 20, 2021

あとleftJoinAndSelect('note.user', 'user')よりは、innerJoinAndSelect('note.user', 'user')なのかも?と
今は、ユーザー→ノートでカスケード削除されるからどっちども同じだけど、ユーザーがいない時はレコードを返さないのが正しいかも。

@mei23
Copy link
Contributor

mei23 commented Mar 21, 2021

syuilo/Misskey.old@d7e7848
note.user 以外はINNERにしちゃダメなはず

@mei23
Copy link
Contributor

mei23 commented Mar 21, 2021

.leftJoinAndSelect('reply.user', 'replyUser')
.leftJoinAndSelect('renote.user', 'renoteUser')

は使われない気がする?

@syuilo
Copy link
Member

syuilo commented Mar 21, 2021

```ts
.leftJoinAndSelect('reply.user', 'replyUser')
.leftJoinAndSelect('renote.user', 'renoteUser')

は使われない気がする?

リプライとRenoteのpackするときは必ずそのユーザーも引っ張ってくるからjoinしてる

@syuilo
Copy link
Member

syuilo commented Mar 21, 2021

d7e7848
note.user 以外はINNERにしちゃダメなはず

リプライとかrenoteが消えていたときも(今のところ)レコード返したくないからそうしたけど、問題ありそうかな

@mei23
Copy link
Contributor

mei23 commented Mar 21, 2021

replyUser, renoteUser はどこからも参照されてないし、entityにも定義なくないです?

@syuilo
Copy link
Member

syuilo commented Mar 21, 2021

確認したときはこの書き方でjoinされたと思ったけど、勘違いだったかな
再度確認してみる

@mei23
Copy link
Contributor

mei23 commented Mar 21, 2021

リプライとかrenoteが消えていたときも(今のところ)レコード返したくないからそうしたけど、問題ありそうかな

最初からリプライでもRenoteでもない投稿がまるごと消えちゃう

@rinsuki
Copy link
Contributor

rinsuki commented Mar 21, 2021

というかappがDBに投げるクエリ数の話なのに INNER JOIN か LEFT JOIN かって関係あるの?

@syuilo
Copy link
Member

syuilo commented Mar 21, 2021

クエリ数で言えば関係ない

@syuilo
Copy link
Member

syuilo commented Mar 21, 2021

確認したときはこの書き方でjoinされたと思ったけど、勘違いだったかな
再度確認してみる

やはりjoinされてたので、第二引数は他のクエリ内で参照するための単なるエイリアス的な扱いでそんなに重要じゃないのかも

@syuilo
Copy link
Member

syuilo commented Mar 22, 2021

出来ることはやって大分減ったはず
もう改修の余地がほぼなく、これ以上は原理的に(?)減らせないと思うのでclose

@syuilo syuilo closed this as completed Mar 22, 2021
@syuilo syuilo unpinned this issue Mar 22, 2021
@SanMurakami
Copy link
Contributor Author

かなり改善しました、ありがとうございます。

改善前
image

改善後
image

foggy1 pushed a commit to foggy-llc/pluskey that referenced this issue May 8, 2021
foggy1 pushed a commit to foggy-llc/pluskey that referenced this issue May 8, 2021
foggy1 pushed a commit to foggy-llc/pluskey that referenced this issue May 8, 2021
snk-lab pushed a commit to snk-lab/misskey-kusa-note that referenced this issue Sep 24, 2021
snk-lab pushed a commit to snk-lab/misskey-kusa-note that referenced this issue Sep 24, 2021
snk-lab pushed a commit to snk-lab/misskey-kusa-note that referenced this issue Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR 🐢Performance Efficiency related issue/PR
Projects
None yet
Development

No branches or pull requests

4 participants