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

perf(backend): Improve performance of FetchInstanceMetadata #11128

Merged

Conversation

yuriha-chan
Copy link
Contributor

@yuriha-chan yuriha-chan commented Jul 6, 2023

#11000

What

  1. インスタンスの情報のDB上の最終更新時刻を確認し、古かったら更新する操作がredis-lockにより排他制御されていたが、ロックを取得できなかった場合にリトライするのをやめ、何もしないようにした。
  2. 「forceでない限り実際に処理を走らせるのは1日以上間隔を開ける」を実現するのにInstanceを取りに行くが、その際PostgreSQLに直接問い合わせるのではなくfederatedInstanceService.fetch経由でRedisのキャッシュを活用する

Why

FetchInstanceMetadata は Deliver Jobから呼ばれるため高速大量に呼ばれるため、これを改めることでパフォーマンスが顕著に改善する。

  1. このロックはインスタンス情報の更新操作が一回だけ行われることを目的としているため、ロックが取得できない場合、何もしないことに問題はない。
  2. どうしてfederatedInstanceService.fetchを使っていなかったのかわからない(使うようにすればPostgreSQLへの問い合わせ回数が相当減るはず)

Additional info (optional)

以下の動作を確認しました。

  • Deliver完了時にInstance Metadataが正しく更新されることを確認した。
  • 大量のDeliverを高速に行ったときに、パフォーマンスが改善することを確認した。

redis-lockにリトライを設定するオプションがなかったので、Redis上にロックとして使うキーを設定し、これをアトミックに参照・書き換える実装とした。

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Jul 6, 2023
@github-actions github-actions bot requested review from syuilo and tamaina July 6, 2023 00:27
@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #11128 (ad65a4f) into develop (f76b3ed) will increase coverage by 0.45%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop   #11128      +/-   ##
===========================================
+ Coverage    77.36%   77.81%   +0.45%     
===========================================
  Files          908      171     -737     
  Lines        91662    21487   -70175     
  Branches      7551      498    -7053     
===========================================
- Hits         70916    16721   -54195     
+ Misses       20746     4766   -15980     

see 737 files with indirect coverage changes

@syuilo
Copy link
Member

syuilo commented Jul 6, 2023

テスト欲しくなってきたわね

@yuriha-chan
Copy link
Contributor Author

連合のテストやパフォーマンスのテストが結構やっかいなのですよね。
いま私が手動&目視でやっているテストは、複数のサーバーを立ち上げてフォローさせ、APIで大量に投稿を投げるというテストです。
(production環境でもうまくいくかどうかを確かめるため、ほぼ本番環境とほぼ同様のサーバーをLAN内でオレオレ証明書で動かしています。NODE_OPTIONS=–use-openssl-ca と (/etc/hostsを使う場合)cacheablelookupの設定変更が必要。)

@syuilo
Copy link
Member

syuilo commented Jul 6, 2023

パフォーマンスのテストはそこまで重要じゃないと思っていて、このPRでいえば「ロックを取得できなかった場合にリトライするのをやめ、何もしないようになった」ことを担保するテストがあると良いかなと思った

@yuriha-chan
Copy link
Contributor Author

redisClientのモックを作ればそういうこともやりやすいかも。

@tamaina
Copy link
Member

tamaina commented Jul 7, 2023

@yuriha-chan 失礼ですが、エディタは何を使われていますか?

@yuriha-chan
Copy link
Contributor Author

vimだけど設定に無頓着なせいで劣悪な環境で書いていたりしています

@tamaina
Copy link
Member

tamaina commented Jul 7, 2023

VS Codeを使ってくださいね。

@yuriha-chan
Copy link
Contributor Author

ぐぬぬ…

@tamaina
Copy link
Member

tamaina commented Jul 7, 2023

(なんかもう全然ダメダメで笑うしかなくなった)

@tamaina tamaina changed the title Perf: Stop retrying to acquire lock for FetchInstanceMetadata perf(backend): Improve performance of FetchInstanceMetadata Jul 7, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
@tamaina tamaina merged commit 4c879b3 into misskey-dev:develop Jul 7, 2023
13 of 16 checks passed
@tamaina
Copy link
Member

tamaina commented Jul 7, 2023

👍 👍👍👍

yu256 pushed a commit to yu256/akatsukey that referenced this pull request Jul 21, 2023
* Perf: Avoid retries to acquire lock in fetchInstanceMetadata

* Fix

* Add Changelog

* Fix typo

* Fix lint

* 記法をMisskey式にする

* ????

* refactor
misskey-dev#11128 (review)

* refactor

* getいらない?

* fix

* fix

* Update CHANGELOG.md

* clean up

---------

Co-authored-by: tamaina <tamaina@hotmail.co.jp>
slofp pushed a commit to Secineralyr/misskey.dream that referenced this pull request Jul 21, 2023
…dev#11128)

* Perf: Avoid retries to acquire lock in fetchInstanceMetadata

* Fix

* Add Changelog

* Fix typo

* Fix lint

* 記法をMisskey式にする

* ????

* refactor
misskey-dev#11128 (review)

* refactor

* getいらない?

* fix

* fix

* Update CHANGELOG.md

* clean up

---------

Co-authored-by: tamaina <tamaina@hotmail.co.jp>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants