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

fix(backend): memory leak in memory caches #14363

Merged
merged 12 commits into from
Aug 18, 2024

Conversation

warriordog
Copy link
Contributor

@warriordog warriordog commented Aug 4, 2024

What

This PR resolves #14310 and #14311 by replacing all "infinite" cache lifetimes and cleaning up MemoryKVCache<T>. Expiration lifetimes of all memory / redis caches have been reset to a reasonable number and normalized between similar caches.
Redis caches have a synchronization fix, and an optimization is applied to MemoryKVCache.gc().

Why

The linked issues pose a serious reliability issue that is affecting many instances, including my own. Resolving the memory leaks will improve stability, uptime, and performance.

Fix #14310
Fix #14311

Additional info (optional)

This change is upstreamed from the Sharkey fork. Please see Sharkey MR #580 for additional details and context.

This replaces #14314, which was an earlier attempt at a similar fix.

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 Aug 4, 2024
Copy link
Contributor

github-actions bot commented Aug 4, 2024

このPRによるapi.jsonの差分

差分はこちら

Get diff files from Workflow Page

Copy link

codecov bot commented Aug 4, 2024

Codecov Report

Attention: Patch coverage is 85.14851% with 15 lines in your changes missing coverage. Please review.

Project coverage is 39.87%. Comparing base (75b0315) to head (5a1ef8d).
Report is 13 commits behind head on develop.

Files Patch % Lines
packages/backend/src/misc/cache.ts 88.60% 9 Missing ⚠️
packages/backend/src/core/CustomEmojiService.ts 50.00% 3 Missing ⚠️
packages/backend/src/core/CacheService.ts 83.33% 1 Missing ⚠️
...ckages/backend/src/server/NodeinfoServerService.ts 0.00% 1 Missing ⚠️
...ages/backend/src/server/api/AuthenticateService.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14363      +/-   ##
===========================================
- Coverage    39.90%   39.87%   -0.03%     
===========================================
  Files         1547     1547              
  Lines       190875   190901      +26     
  Branches      3569     2655     -914     
===========================================
- Hits         76166    76123      -43     
- Misses      114116   114216     +100     
+ Partials       593      562      -31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kakkokari-gtyih kakkokari-gtyih changed the title fix memory leak in memory caches fix(backend): memory leak in memory caches Aug 4, 2024
Co-authored-by: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com>
@u1-liquid
Copy link
Sponsor Contributor

See also: MisskeyIO#174 MisskeyIO#289

@syuilo syuilo merged commit 9ce44b2 into misskey-dev:develop Aug 18, 2024
26 checks passed
@syuilo
Copy link
Member

syuilo commented Aug 18, 2024

🙏🏻

LemonDouble pushed a commit to LemonDouble/misskey that referenced this pull request Aug 19, 2024
* encapsulate `MemoryKVCache<T>`

* remove infinity caches

* encapsulate other caches

* add missing awaits to internally synchronize caches

* implement pull-through caching

* tune cache lifetimes

* optimize cache GC by stopping early

* summarize changes in CHANGELOG.md

* Fix timeout comments

Co-authored-by: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com>

* add comments about awaiting the redis write

---------

Co-authored-by: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com>
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
4 participants