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

Debounce SQL in note pack #176

Merged
merged 2 commits into from
Oct 7, 2023
Merged

Conversation

KOBA789
Copy link
Collaborator

@KOBA789 KOBA789 commented Oct 7, 2023

What

  • PubSub の notesStream のイベント受信時のクエリ発行数を削減し、app/DB 共に負荷を削減します
  • 同 microtask 内で呼び出された notesRepository.findOneOrFail をバッファリングし、同じ id へのクエリを1つにまとめます(debounce)
  • debounce 処理を行う DebounceLoader を実装し、追加します

Why

PubSub から notesStream のイベントを受信すると、そのプロセスが担当しているクライアントのコネクション数の分だけ NoteEntityService.pack が呼び出されます。
(正確には、その note に replyId や renoteId が付いている場合)
つまり、同一 noteId に対するクエリがクライアント数と同じ量発射されます。
当然これはあまりに無駄で、実際に WebSocket を担当するプロセスの CPU 使用率が断続的に数秒間100%に貼り付くといったパフォーマンスの問題も起きているため、これを解消したいと考えました。

Additional info (optional)

NoteEntityService のテストが困難なので、ちょっと検証が足りてないかも。
DebounceLoader のユニットテストは書いたし通ったので、たぶん動く……

@KOBA789 KOBA789 changed the title Debounce note pack Debounce SQL in note pack Oct 7, 2023
@sonarcloud
Copy link

sonarcloud bot commented Oct 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@KOBA789 KOBA789 merged commit 99e14d3 into MisskeyIO:io Oct 7, 2023
16 checks passed
@KOBA789 KOBA789 deleted the debounce-note-pack branch October 7, 2023 17:42
Comment on lines +490 to +491
private async findNoteOrFail(id: string): Promise<MiNote> {
return await this.notesRepository.findOneOrFail({
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion:

private async findNoteOrFail(id: string): Promise<MiNote> {
		return await this.notesRepository.findOneOrFail({

private findNoteOrFail(id: string): Promise<MiNote> {
		return this.notesRepository.findOneOrFail({

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants