github-starred/pangolin
2
0
Fork 0
mirror of https://github.com/fosrl/pangolin.git synced 2026-05-06 20:59:07 -05:00
Code Issues 564 Packages Projects Releases 72 Wiki Activity

[PR #2133] [MERGED] Fix: Prevent cache memory leak with maxKeys limit and conditional caching #9658

New Issue
Closed
opened 2026-04-30 05:50:23 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-04-30 05:50:23 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/fosrl/pangolin/pull/2133
Author: @djcrafts
Created: 12/21/2025
Status: ✅ Merged
Merged: 12/21/2025
Merged by: @oschwartz10612

Base: dev ← Head: fix/cache-memory-leak

📝 Commits (1)

  • 2add9db Fix: Prevent cache memory leak by adding maxKeys limit and conditional caching

📊 Changes

3 files changed (+26 additions, -5 deletions)

View changed files

📝 server/lib/cache.ts (+16 -1)
📝 server/private/lib/logAccessAudit.ts (+5 -2)
📝 server/routers/badger/verifySession.ts (+5 -2)

📄 Description

Summary

Fixes #2120 by preventing unbounded cache growth through three key improvements:

  1. Adding a maxKeys limit to the NodeCache instance
  2. Skipping cache writes for unsuccessful lookups
  3. Adding periodic cache statistics logging for monitoring

Problem

After upgrading to 1.13.1, Pangolin was experiencing memory leaks, especially when GeoIP (MaxMind) database was configured. With ~10k requests per day, the cache would grow unbounded, eventually causing OOM kills on memory-constrained systems.

Root causes:

  1. NodeCache had no maxKeys limit, allowing unlimited growth
  2. GeoIP/ASN lookups were cached even when they returned undefined (e.g., when MaxMind DB not configured)
  3. No visibility into cache performance made the issue hard to diagnose

Solution

1. Add maxKeys limit (cache.ts)

export const cache = new NodeCache({
    stdTTL: 3600,
    checkperiod: 120,
    maxKeys: 10000  // Prevents unbounded growth
});
  • Sets maximum of 10,000 cached keys
  • Uses LRU eviction when limit is reached
  • With ~10k requests/day and 5min TTL, provides ample headroom

2. Skip caching undefined values

Before:

cachedCountryCode = await getCountryCodeForIp(ip);
cache.set(geoIpCacheKey, cachedCountryCode, 300); // Caches undefined too

After:

cachedCountryCode = await getCountryCodeForIp(ip);
if (cachedCountryCode) {  // Only cache successful lookups
    cache.set(geoIpCacheKey, cachedCountryCode, 300);
}

3. Add cache monitoring

setInterval(() => {
    const stats = cache.getStats();
    logger.debug(
        `Cache stats - Keys: ${stats.keys}, Hits: ${stats.hits}, Misses: ${stats.misses}, Hit rate: ...`
    );
}, 300000); // Every 5 minutes

Testing

  • ✅ TypeScript compilation successful (no type errors)
  • ✅ No functional changes to lookup logic
  • ✅ Backward compatible with existing cache usage
  • ✅ Cache statistics will be visible in debug logs

Impact

  • Memory usage: Capped at ~10k keys instead of unbounded growth
  • Performance: No degradation; LRU ensures frequently accessed IPs remain cached
  • Monitoring: Cache stats in logs help identify issues early
  • Undefined caching: Prevents wasting cache space on failed lookups

Related

  • Addresses issue raised by @Ragnaruk about lack of cache stats logging
  • Addresses issue raised by @nath1416 about OOM kills on 1GB VPS
  • ASN lookup caching already had conditional caching; now GeoIP matches that pattern

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/fosrl/pangolin/pull/2133 **Author:** [@djcrafts](https://github.com/djcrafts) **Created:** 12/21/2025 **Status:** ✅ Merged **Merged:** 12/21/2025 **Merged by:** [@oschwartz10612](https://github.com/oschwartz10612) **Base:** `dev` ← **Head:** `fix/cache-memory-leak` --- ### 📝 Commits (1) - [`2add9db`](https://github.com/fosrl/pangolin/commit/2add9db236c679b6407221bf25452e198f18de34) Fix: Prevent cache memory leak by adding maxKeys limit and conditional caching ### 📊 Changes **3 files changed** (+26 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `server/lib/cache.ts` (+16 -1) 📝 `server/private/lib/logAccessAudit.ts` (+5 -2) 📝 `server/routers/badger/verifySession.ts` (+5 -2) </details> ### 📄 Description ### Summary Fixes #2120 by preventing unbounded cache growth through three key improvements: 1. Adding a `maxKeys` limit to the NodeCache instance 2. Skipping cache writes for unsuccessful lookups 3. Adding periodic cache statistics logging for monitoring ### Problem After upgrading to 1.13.1, Pangolin was experiencing memory leaks, especially when GeoIP (MaxMind) database was configured. With ~10k requests per day, the cache would grow unbounded, eventually causing OOM kills on memory-constrained systems. **Root causes:** 1. NodeCache had no `maxKeys` limit, allowing unlimited growth 2. GeoIP/ASN lookups were cached even when they returned `undefined` (e.g., when MaxMind DB not configured) 3. No visibility into cache performance made the issue hard to diagnose ### Solution #### 1. Add maxKeys limit (cache.ts) ```typescript export const cache = new NodeCache({ stdTTL: 3600, checkperiod: 120, maxKeys: 10000 // Prevents unbounded growth }); ``` - Sets maximum of 10,000 cached keys - Uses LRU eviction when limit is reached - With ~10k requests/day and 5min TTL, provides ample headroom #### 2. Skip caching undefined values **Before:** ```typescript cachedCountryCode = await getCountryCodeForIp(ip); cache.set(geoIpCacheKey, cachedCountryCode, 300); // Caches undefined too ``` **After:** ```typescript cachedCountryCode = await getCountryCodeForIp(ip); if (cachedCountryCode) { // Only cache successful lookups cache.set(geoIpCacheKey, cachedCountryCode, 300); } ``` #### 3. Add cache monitoring ```typescript setInterval(() => { const stats = cache.getStats(); logger.debug( `Cache stats - Keys: ${stats.keys}, Hits: ${stats.hits}, Misses: ${stats.misses}, Hit rate: ...` ); }, 300000); // Every 5 minutes ``` ### Testing - ✅ TypeScript compilation successful (no type errors) - ✅ No functional changes to lookup logic - ✅ Backward compatible with existing cache usage - ✅ Cache statistics will be visible in debug logs ### Impact - **Memory usage**: Capped at ~10k keys instead of unbounded growth - **Performance**: No degradation; LRU ensures frequently accessed IPs remain cached - **Monitoring**: Cache stats in logs help identify issues early - **Undefined caching**: Prevents wasting cache space on failed lookups ### Related - Addresses issue raised by @Ragnaruk about lack of cache stats logging - Addresses issue raised by @nath1416 about OOM kills on 1GB VPS - ASN lookup caching already had conditional caching; now GeoIP matches that pattern --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-30 05:50:23 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
api
authentication
bug
config
dependencies
docker
documentation
enhancement
good first issue
help wanted
Improvement
Look Into
needs investigating
networking
new feature
non-critical bug
potential bug
pull-request
Mirrored from GitHub Pull Request
 question
reverse proxy
Security
stale
ui
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/pangolin#9658
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?