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

Hot-fix: Write-back value in memory when it's available in redis #174

Merged
merged 3 commits into from
Oct 6, 2023

Conversation

KOBA789
Copy link
Collaborator

@KOBA789 KOBA789 commented Oct 6, 2023

What

Memory 上の lifetime(TTL) は切れたが、Redis 上の lifetime はまだ切れていないという状況で、Redis から読み取った値を MemoryCache に書き戻します。

Why

現状、Redis + MemoryCache の2層キャッシュである RedisSingleCache において、MemoryCache 上の lifetime(TTL) は切れたが、Redis 上の lifetime はまだ切れていないという状況で、都度 Redis へのリクエストと Redis からのレスポンスのパースが走ってしまいます。
(つまり、MemoryCache が仕事をしなくなってしまう)
この影響で、CustomEmojiService ではキャッシュを書いた3分後からの27(=30-3)分間は Redis へのクエリと JSON.parse が大爆発するボーナスタイムになっています。
このパッチはこれを解決します。

Additional info (optional)

Redis 上での残り lifetime を考慮せずに MemoryCache に書き戻してしまうため、実際の lifetime が lifetime + memoryCacheLifetime となります。
たとえば CustomEmojiService では従来30分間の lifetime だったところ、このパッチの影響で33分間キャッシュされることになります。

Copy link
Collaborator

@riku6460 riku6460 left a comment

Choose a reason for hiding this comment

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

これ memoryCache から取ってきたものも set してしまい TTL が無限みたいなことになりません?

u1-liquid
u1-liquid previously approved these changes Oct 6, 2023
@KOBA789
Copy link
Collaborator Author

KOBA789 commented Oct 6, 2023

@riku6460 You're true.

Copy link
Collaborator

@riku6460 riku6460 left a comment

Choose a reason for hiding this comment

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

おそらく RedisKVCache も同じ問題を抱えてそうですが、どうしましょうか

@u1-liquid
Copy link
Member

resolvedにされてるのみてなかった

MemorySingleCacheのcachedAtだけリセットしたい気持ちがあります

@KOBA789
Copy link
Collaborator Author

KOBA789 commented Oct 6, 2023

今ハッキリと爆発しているのは CustomEmojiService なので、一旦これ入れて結果を見たいかなという気持ちです。
無駄に影響範囲広げたくないなぁと。

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 6, 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
0.0% 0.0% Duplication

@riku6460
Copy link
Collaborator

riku6460 commented Oct 6, 2023

確かに。とりあえずここでは RedisSingleCache のみで良さそうです

@KOBA789 KOBA789 requested a review from riku6460 October 6, 2023 13:32
@u1-liquid u1-liquid merged commit 533bae1 into io Oct 6, 2023
17 checks passed
@u1-liquid u1-liquid deleted the fix-read-through branch October 6, 2023 13:42
nafu-at pushed a commit to TeamNijimiss/nijimiss_legacy that referenced this pull request Jul 17, 2024
…skeyIO#174)

* Write-back value in memory when it's available in redis

* Fix recursive cache

* Fix bug in case that invalid data in redis cache
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