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

NoteCreateService.postNoteCreated()をワーカープロセスで動かせるオプションの追加 #15052

Open
1 task
samunohito opened this issue Nov 24, 2024 · 8 comments
Labels
✨Feature This adds/improves/enhances a feature

Comments

@samunohito
Copy link
Member

Summary

ノートそのものを作成した後に呼び出される、通知やTLへのpushなどを行う諸処理が実装されたpostNoteCreated()なる関数があります。この関数は多くの機能を持ち、それに応じて負荷も高いです。

現在は以下のようにsetImmediate()を活用して早くレスポンス出来るようにされています。…が、メインプロセスで処理されるのは変わりないので、負荷を落とせるわけではありません。

setImmediate('post created', { signal: this.#shutdownController.signal }).then(
() => this.postNoteCreated(note, user, data, silent, tags!, mentionedUsers!),
() => { /* aborted, ignore this */ },
);

そこで、setImmediate()の代わりとしてジョブキューに要求を追加し、ワーカープロセス側でpostNoteCreated()の処理を行う方式を提案したいです(わりと大きく仕組みが変わるので、最初はexperimental的オプションとして提供したい)

実際に、10ms間隔でノートを作成するスクリプトを動かして試したところ、以下のような結果が見えてきました。

  • 素の状態:7本並列起動でメインプロセスの処理速度が落ち始める
  • ジョブキューに回す実装:14本並列起動でメインプロセスの処理速度が落ち始める

※処理速度が落ち始める=テスト用のスクリプトに帰ってくるリクエスト・レスポンスの間隔が開き、リクエストのペースが落ち始める

もちろん、ワーカープロセスの負荷が増すのでサーバによっては不向きな機能かもしれませんが(故にオプション)、メインプロセスが他のリクエストにパワーを回せるようになるので有用な選択肢かと思いました。

Purpose

  • メインプロセスの負荷を落とし、捌けるリクエストの数が増える

Do you want to implement this feature yourself?

  • Yes, I will implement this by myself and send a pull request
@samunohito samunohito added the ✨Feature This adds/improves/enhances a feature label Nov 24, 2024
@syuilo
Copy link
Member

syuilo commented Nov 26, 2024

APIのリクエストを捌くのはそもそもワーカープロセスだったような?

@syuilo
Copy link
Member

syuilo commented Nov 26, 2024

「メインプロセス」「ワーカープロセス」がそれぞれ何を指しているのかの認識が合っていないかもしれない

@samunohito
Copy link
Member Author

あー。おさむ語である可能性が無きにしも非ず…

自分は

メインプロセス:↓こっちで起動されるほう(node ./built/boot/entry.jsで起動されたプロセスと同一の認識)

if (cluster.isPrimary || envOption.disableClustering) {
await masterMain();
if (cluster.isPrimary) {
ev.mount();
}
}

ワーカープロセス:↓こっちで起動される方

if (cluster.isWorker || envOption.disableClustering) {
await workerMain();
}

の認識でいました

@syuilo
Copy link
Member

syuilo commented Nov 26, 2024

あー。おさむ語である可能性が無きにしも非ず…

自分は

メインプロセス:↓こっちで起動されるほう(node ./built/boot/entry.jsで起動されたプロセスと同一の認識)

misskey/packages/backend/src/boot/entry.ts

Lines 71 to 77 in d55e638

if (cluster.isPrimary || envOption.disableClustering) {
await masterMain();

if (cluster.isPrimary) {
ev.mount();
}
}
ワーカープロセス:↓こっちで起動される方

misskey/packages/backend/src/boot/entry.ts

Lines 79 to 81 in d55e638

if (cluster.isWorker || envOption.disableClustering) {
await workerMain();
}
の認識でいました

合ってた

APIのリクエストを捌くのはそもそもワーカープロセスだったような?

そんなことなかった

@syuilo
Copy link
Member

syuilo commented Nov 26, 2024

APIリクエストを捌くの自体をワーカープロセスに移動した方が良い可能性がある可能性がある

@samunohito
Copy link
Member Author

APIリクエストを捌くの自体をワーカープロセスに移動した方が良い可能性がある

related to #13662

@samunohito
Copy link
Member Author

↑をサポートしたとしても、postNoteCreated()を切り離すこと自体は有用であると考えています。
APIリクエストを捌くワーカープロセス、それ以外の諸処理を行うワーカープロセスみたいな感じで役割分担も可能でしょうし…(APIサーバとしてのモジュールをメモリに読み込むか、ジョブキューとしてのモジュールを読み込むかで使用メモリとかも違ってきそう)

@syuilo
Copy link
Member

syuilo commented Nov 26, 2024

ほむん

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨Feature This adds/improves/enhances a feature
Projects
Development

No branches or pull requests

2 participants