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

リアクションごとにノートのリアクション情報が更新されるのは重い #11093

Closed
syuilo opened this issue Jul 3, 2023 · 41 comments · Fixed by #14579
Closed
Assignees
Labels
🔥high priority packages/backend Server side specific issue/PR 🐢Performance Efficiency related issue/PR

Comments

@syuilo
Copy link
Member

syuilo commented Jul 3, 2023

Summary

Issueあった気がしたけど見当たらなかった

例えば1秒間に100回リアクションがつく場合リアクションレコードの挿入に加えて、100回ノートレコードの更新(=PostgreSQLでは実質的にノートテーブルの新規挿入&削除)が走ることになりかなり負荷が高い

そのためノートレコードごとにリアクション情報を非正規化して持つのをやめたいが、ノート取得するたびにリアクションを集計してくるのもかなり負荷が高いと思われるので、何かいい解決策を考える必要がある

例えばRedisにうまいことキャッシュできないかしら

  • Redisにキャッシュして定期的にDBにベイクする(つまりRedis→DB)
    • 懸念: 結局ベイクが重そう、実装が難しそう
  • Redisにキャッシュしてキャッシュがある場合はそれを使い、ない場合はDBから集計+キャッシュ追加(つまりDB→Redis)
    • 懸念: DBから集計が重そう
  • 非正規化リアクション情報用DBを別に用意できるようにする
@syuilo syuilo added 🐢Performance Efficiency related issue/PR packages/backend Server side specific issue/PR labels Jul 3, 2023
@syuilo
Copy link
Member Author

syuilo commented Jul 3, 2023

もしくは非正規化して持っててもいいけど、更新は定期的にまとめて行うようにするとか

@syuilo
Copy link
Member Author

syuilo commented Jul 3, 2023

SlackとかDiscordがどういう実装してるのか気になるな

@yuriha-chan
Copy link
Contributor

投稿とかリノートされた時にブワーってつく傾向にあるから、Redisで短期間のキャッシュを持ち、反応が落ち着いたらDBに反映というのが良さそう

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

問題は、RedisはJSONに対応してないからオブジェクトの特定のキーの値をアトミックにインクリメントするみたいな処理がおそらく出来ないこと

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

reactionsCache:<noteId>:<reaction>
みたいなキーで持っておけば良いかも

@syuilo syuilo self-assigned this Jul 4, 2023
@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

  1. Redisにキーを追加する
  2. 定期的にRedisからキーを読み出し、DBに反映する
  3. Redisからキーを削除する

という流れになると思うけど、2を処理している最中に新しくキーが追加されたらそれが処理されないまま3に進んでキーが削除され、間違った値がDBにベイクされてしまう問題が発生するなど、意外と面倒かも

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

良い実装方法募集中

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

あとノートの数が多い場合、今よりだいぶマシになるとはいえやっぱり2の負荷が高そう
それを考えると究極的には「非正規化して持つのをやめる」しないといけないんだけどなかなか難しい

@tamaina
Copy link
Contributor

tamaina commented Jul 4, 2023

PostgreSQLにおいて、あるレコードに対して1秒間に大量の書き換えがある場合の、一般的なパフォーマンス改善策ってなんだろう

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

ノート取得するたびにリアクションを集計してくるのもかなり負荷が高いと思われる

クエリの書き方によっては別にそうでもない可能性はあるな

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

Redisにキャッシュしつつ、Redisにキャッシュがない場合だけDBから集計する感じでいけないかな

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

reactionsテーブルに対してノートIDの配列を渡したら以下のような結果が返ってくるSQL募集中

[{
  noteId: "hoge",
  reactions: {
    'foo': 10,
    'bar': 5,
  }
}, {
  noteId: "huga",
  reactions: {
    'foo': 20,
    'buzz': 4,
    'bar': 2,
  }
}, {
  ...
}]

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

投稿日時が今からT以内の投稿であれば、リアクションされていれば高確率でRedisにキャッシュされているはずなのでRedisからキャッシュを読み、全面的にそれを採用しRedisになかったとしてもDBには問い合わせない(「その投稿にリアクションがされていない」ことと「リアクションされているがRedisにキャッシュされていない」ことの区別が不可能なため、パフォーマンスを優先しRedisにない場合はリアクションされていないと見做す)
投稿日時がTより前の投稿であれば、Redisにキャッシュされていないかキャッシュが消えている可能性が高まってくるため、Redisにキャッシュがあるか確認し、なければDBから集計+キャッシュ更新

が良いかも
ここで T = 3時間とか?

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

問題としては

投稿日時がTより前の投稿であれば、Redisにキャッシュがあるか確認し、なければDBから集計+キャッシュ更新

の負荷が高い可能性があるという点

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

実際にいっぱいリアクション付いた投稿に対してreactionsテーブルに対してノートIDの配列を渡したら以下のような結果が返ってくるSQLを投げてみてどれくらいの負荷になるか確認したい

@tamaina
Copy link
Contributor

tamaina commented Jul 4, 2023

以下のような結果が

SELECT "noteId", "reaction", COUNT(*) AS "reactionCount"
FROM note_reaction
WHERE "noteId" IN ('9dh0bkw15q', '9dmmip9y48')
GROUP BY "noteId", "reaction";

[noteId, reaction, reactionCount][]が返ってくるけどそれ以上は私はわからなかった(しSQLの仕事ではないかも?)

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

で[noteId, reactionName, reactionCount][]が返ってくる

よさそう

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

同じ種類のリアクション100個にたいして100レコードが返ってくるというのを避けられればどんな形式でも良い

@tamaina
Copy link
Contributor

tamaina commented Jul 4, 2023

でもCOUNTって絶対遅いよね

@syuilo
Copy link
Member Author

syuilo commented Jul 4, 2023

インデックスあっても遅いかしら

@tamaina
Copy link
Contributor

tamaina commented Jul 4, 2023

note_reactionが762万レコードあるけど↑のようなSQLだと一瞬だったわ(INだと流石にインデックスが効いてるか)

@syuilo syuilo changed the title ノートのレコードごとにリアクション情報を非正規化して持つのをやめる リアクションごとにノートのリアクション情報が更新されるのは重い Jul 4, 2023
@yuriha-chan
Copy link
Contributor

問題は、RedisはJSONに対応してないからオブジェクトの特定のキーの値をアトミックにインクリメントするみたいな処理がおそらく出来ないこと

できるっぽい
https://redis.io/commands/hincrby/

@syuilo
Copy link
Member Author

syuilo commented Jul 5, 2023

以下のような結果が

SELECT "noteId", "reaction", COUNT(*) AS "reactionCount"
FROM note_reaction
WHERE "noteId" IN ('9dh0bkw15q', '9dmmip9y48')
GROUP BY "noteId", "reaction";

[noteId, reaction, reactionCount][]が返ってくるけどそれ以上は私はわからなかった(しSQLの仕事ではないかも?)

ioのDBでめちゃくちゃリアクション付いてる投稿とかめちゃくちゃ古い投稿とか全くリアクション付いてない投稿などいろいろ指定してクエリしてみてどれくらい時間かかるか計測しようとしている

@syuilo
Copy link
Member Author

syuilo commented Jul 5, 2023

ちなみにioではnote_reactionレコードは1億くらいあるっぽい

@tamaina
Copy link
Contributor

tamaina commented Jul 5, 2023

(ちなみにSQLはChatGPT頼り)

@pumpkin2048b
Copy link

pumpkin2048b commented Jul 5, 2023

PostgreSQLにおいて、あるレコードに対して1秒間に大量の書き換えがある場合の、一般的なパフォーマンス改善策ってなんだろう

#10173
ここらへんで触れてますが、1度の処理で書き込める状況ならBulk updateがいいかと思われます。
一般的なデータ投入でもパフォーマンスの改善として使われる手法だと認識してます。

@syuilo
Copy link
Member Author

syuilo commented Jul 5, 2023

ioのDBでめちゃくちゃリアクション付いてる投稿とかめちゃくちゃ古い投稿とか全くリアクション付いてない投稿などいろいろ指定してクエリしてみてどれくらい時間かかるか計測しようとしている

150msくらいだった
この速度だとちょっと現実的ではないな(数十msであってほしい)

@syuilo
Copy link
Member Author

syuilo commented Jul 5, 2023

今後もリアクションのレコード数は増加していくことを考えると、やっぱりノートからreactionsを消すことは現実的ではなさそうで、「noteにreactionsは持ちつつもその更新は一定間隔でbulk updateする」が落としどころかもなあ

@syuilo
Copy link
Member Author

syuilo commented Jul 5, 2023

おごごごご

@tamaina
Copy link
Contributor

tamaina commented Jul 5, 2023

Redisのリアクションイベントを拾って一定間隔(実行完了から1秒後みたいなインターバルにする?)でDBに適用するというDBキューがあればいいのか

@tamaina
Copy link
Contributor

tamaina commented Jul 5, 2023

リアクションイベントをメモリに保持しておく必要があるんだからキューじゃダメでデーモンじゃないとか

@MineCake147E

This comment was marked as off-topic.

@syuilo
Copy link
Member Author

syuilo commented Jul 5, 2023

それが今の状態ですね

@syuilo
Copy link
Member Author

syuilo commented Jul 5, 2023

あーテーブルは同じか

@syuilo
Copy link
Member Author

syuilo commented Jul 5, 2023

テーブル分けたとしても結局そのテーブルの更新が重くなる

@tamaina
Copy link
Contributor

tamaina commented Jul 5, 2023

テーブルを分けるならそれをRedisに保持しておくほうがいいんじゃないかと(整合性は知らない)

@yuriha-chan
Copy link
Contributor

要するに以下のような要求がある。
「アクティブ状態」のときにはRedisのHashを参照すれば正確なカウントが得られる。
「非アクティブ状態」ときにはPostgreSQLの非正規化テーブルを参照すれば正確なカウントが得られる。

これを実現するには以下のようにすればよいはず。

読み出し方法
Redis にキーが存在するかを確認し、存在すればRedisから読み、存在しなければPostgreSQLの非正規化テーブルを読む。

書き込み方法
書き込み時にRedisのhashreactions:<noteId>が存在しない場合は、ノートの年齢がT未満なら、Redisのハッシュテーブルを作成する。そうでないなら、非正規化テーブルをRedisへとコピーする。
Redis のhashのリアクションカウントをインクリメントし、hash全体のexpireをTに更新する。
expireする前に余裕をもって、PostgreSQLの非正規化テーブルへ値をコピーする操作をスケジュールする。すでにその操作が予定されている場合は、その操作を延期する。
(特殊な事情により、書き込み操作を行う前にRedisのhashreactions:<noteId>がexpireしてしまっていたら、このスケジュールされた操作は、PostgreSQLの正規化テーブルから非正規化テーブルを更新する。)

この方法の肝は、PostgreSQLの非正規化テーブルへ値をコピーする操作の後にリアクションが入って、Redisのexpireが延期された(「Redisからの削除がやっぱりなしで」になった)としても、PostgreSQLの非正規化テーブルが予定より早く更新されたというだけで、全体の不整合を引き起こさない点にあります。

@syuilo
Copy link
Member Author

syuilo commented Jul 6, 2023

Redisにキャッシュして定期的にDBにベイクする(つまりRedis→DB)

PostgreSQL(というかTypeORM)、それぞれ異なる値でのbulk updateに対応してない可能性があるな

@syuilo
Copy link
Member Author

syuilo commented Jul 7, 2023

やっぱり対応してなかった
typeorm/typeorm#7326

@syuilo
Copy link
Member Author

syuilo commented Sep 16, 2024

やる必要がある

@syuilo
Copy link
Member Author

syuilo commented Sep 18, 2024

問題は、RedisはJSONに対応してないからオブジェクトの特定のキーの値をアトミックにインクリメントするみたいな処理がおそらく出来ないこと

hash使えそう

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔥high priority packages/backend Server side specific issue/PR 🐢Performance Efficiency related issue/PR
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants