Perf: Stop retrying to acquire lock for FetchInstanceMetadata
What
Fixes #11000 インスタンスの情報のDB上の最終更新時刻を確認し、古かったら更新する操作がredis-lockにより排他制御されていたが、ロックを取得できなかった場合にリトライするのをやめ、何もしないようにした。
Why
FetchInstanceMetadata は Deliver Jobから呼ばれるため高速大量に呼ばれるため、これを改めることでパフォーマンスが顕著に改善する。 このロックはインスタンス情報の更新操作が一回だけ行われることを目的としているため、ロックが取得できない場合、何もしないことに問題はない。
Additional info (optional)
以下の動作を確認しました。
- Deliver完了時にInstance Metadataが正しく更新されることを確認した。
- 大量のDeliverを高速に行ったときに、パフォーマンスが改善することを確認した。
redis-lockにリトライを設定するオプションがなかったので、Redis上にロックとして使うキーを設定し、これをアトミックに参照・書き換える実装とした。
Checklist
- [x] Read the contribution guide
- [x] Test working in a local environment
- [ ] (If needed) Add story of storybook
- [x] (If needed) Update CHANGELOG.md
- [ ] (If possible) Add tests
Codecov Report
Merging #11128 (ad65a4f) into develop (f76b3ed) will increase coverage by
0.45%. The diff coverage isn/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
テスト欲しくなってきたわね
連合のテストやパフォーマンスのテストが結構やっかいなのですよね。
いま私が手動&目視でやっているテストは、複数のサーバーを立ち上げてフォローさせ、APIで大量に投稿を投げるというテストです。
(production環境でもうまくいくかどうかを確かめるため、ほぼ本番環境とほぼ同様のサーバーをLAN内でオレオレ証明書で動かしています。NODE_OPTIONS=–use-openssl-ca と (/etc/hostsを使う場合)cacheablelookupの設定変更が必要。)
パフォーマンスのテストはそこまで重要じゃないと思っていて、このPRでいえば「ロックを取得できなかった場合にリトライするのをやめ、何もしないようになった」ことを担保するテストがあると良いかなと思った
redisClientのモックを作ればそういうこともやりやすいかも。
@yuriha-chan 失礼ですが、エディタは何を使われていますか?
vimだけど設定に無頓着なせいで劣悪な環境で書いていたりしています
VS Codeを使ってくださいね。
ぐぬぬ…
(なんかもう全然ダメダメで笑うしかなくなった)
👍 👍👍👍