feat(config): add LRU eviction#1227
Conversation
floatpanebot
left a comment
floatpanebot
left a comment
There was a problem hiding this comment.
Hi @Haroka-74! Please fix the following issues with your PR:
- Title: Is too long (50 characters). The PR title must be strictly under 40 characters.
floatpanebot
left a comment
floatpanebot
left a comment
There was a problem hiding this comment.
Hi @Haroka-74! Please fix the following issues with your PR:
- Title: Is too long (50 characters). The PR title must be strictly under 40 characters.
mavonx
changed the title
floatpanebot
dismissed stale reviews from themself
Formatting issues have been resolved. Thank you!
|
@Haroka-74 threshold should be configurable in the config. Let the default remain 500mb. but add it in config. Also, if you can, edit the public docs to say how to configure it and what the key in |
|
Sounds good, I’ll sort it out tonight. |
i'll check the PR later, quite busy today. |
|
@andrinoff Done. |
|
|
||
| body.CachedAt = time.Now() | ||
| body.LastAccessedAt = time.Now() | ||
| body.SizeBytes = len(body.Body) |
There was a problem hiding this comment.
this does not count CachedAttachments, while it should. Will lead to a much larger cache, than set in config.
| if calculateTotalCacheSize(cache)+body.SizeBytes > threshold { | ||
| evict(cache, body.SizeBytes, threshold) | ||
| } | ||
|
|
||
| cache.Bodies = append(cache.Bodies, body) |
There was a problem hiding this comment.
If one new email is more than the threshold, evict will drain the cache and still append.
| Body string `json:"body"` | ||
| Attachments []CachedAttachment `json:"attachments,omitempty"` | ||
| CachedAt time.Time `json:"cached_at"` | ||
| LastAccessedAt time.Time `json:"last_accessed_at"` |
There was a problem hiding this comment.
As far as I see, this is never updated. Should be updated on GetCachedEmailBody.
| @@ -522,6 +544,11 @@ func SaveEmailBody(folderName string, body CachedEmailBody) error { | |||
| } | |||
| } | |||
| if !found { | |||
There was a problem hiding this comment.
The least of my concerns, but also worth noting, that, if the email is changed (found=true still) the cache is not going to be evicted
|
/approve |
floatpanebot
left a comment
floatpanebot
left a comment
There was a problem hiding this comment.
Approved on behalf of @andrinoff via /approve command.
3 participants
What?
Implemented a size-based LRU (Least Recently Used) eviction policy for the email body cache.
Why?
The cache grows without bound, reaching hundreds of
MBorGBafter heavy use. This causes disk quota issues on devices with small storage.Closes #1171