Skip to content
Snippets Groups Projects

Draft: fix memory queues to prevent memory leak

Closed Hazelnoot requested to merge fEmber/Sharkey:hazelnoot/fix-memory-queues into develop

This PR has been replaced by !587 (merged).


What does this PR do? (Please give us a brief description of what this PR does.)

This PR resolves #600 (closed) and #601 (closed) by re-implementing MemoryKVCache<T>. The new implementation supports capacity limits with Least Recently Used (LRU) eviction. All references to this class have been updated to specify a reasonable capacity limit.

Additional changes have been made to prevent memory leaks and improve reliability:

  • All "infinite" cache lifetimes have been replaced with a reasonable limit. This includes those that do not use MemoryKVCache<T> directly.
  • The RedisKVCache<T> wrapper class is updated with a new option memoryCacheCapacity. This limits the internal memory cache capacity.
  • MemoryKVCache<T> no longer uses garbage collection, and instead starts an independent timer for each entry. This avoids a costly cache sweep and increases eviction accuracy (from 30% to ~0% eviction latency).
  • MemoryKVCache<T> no longer requires disposal.

Contribution Guidelines By submitting this merge request, you agree to follow our Contribution Guidelines

  • I agree to follow this project's Contribution Guidelines
  • I have made sure to test this pull request

Alternate version for Misskey: https://github.com/warriordog/misskey/tree/hazelnoot/fix-memory-caches

This PR is an untested draft!

Open questions:

  • Should these limits be configurable? Yes, settings would be good.
  • Should this be upstreamed? Yes, change will be upstreamed.
  • Does this work reliably? (needs testing)
  • Is the new eviction method effective? (needs testing)
Edited by Hazelnoot

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading