github-starred/pangolin
2
0
Fork 0
mirror of https://github.com/fosrl/pangolin.git synced 2026-07-15 21:31:24 -05:00
Code Issues 1.6k Packages Projects Releases 79 Wiki Activity

[PR #3138] [CLOSED] perf: memory and CPU remediation for small/resource-constrained instances (#2120) #21473

New Issue
Closed
opened 2026-05-29 02:53:52 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-05-29 02:53:52 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/fosrl/pangolin/pull/3138
Author: @Josh-Voyles
Created: 5/23/2026
Status: ❌ Closed

Base: dev ← Head: mem-tune-2-oc

📝 Commits (10+)

  • 1cdd7db remove mmap_size n cache_size - perf issues small instances
  • daafccb fix: throttle expensive OLM ping operations to once per 5 minutes (#2120)
  • 7001ad8 fix: add statusHistory table cleanup with 90-day retention (#2120)
  • c561ba5 fix: increase TraefikConfigManager default interval to 30s (#2120)
  • f03d690 fix: add TTL to Docker container cache entries (#2120)
  • 16bcc55 fix: remove unnecessary autoFinalizeStatement wrapper (#2120)
  • ab7c61c fix: restore immediate block enforcement on OLM ping path (#2120)
  • 1f8fddf fix: select full client row for sendOlmSyncMessage (#2120)
  • a248a21 revert: per-ping block check on OLM ping path (#2120)
  • 07f7151 feat: push-based OLM termination on block/delete (#2120)

📊 Changes

9 files changed (+160 additions, -152 deletions)

View changed files

📝 server/db/sqlite/driver.ts (+2 -48)
📝 server/lib/cleanupLogs.ts (+10 -3)
📝 server/lib/readConfigFile.ts (+1 -1)
📝 server/routers/client/blockClient.ts (+11 -7)
📝 server/routers/client/deleteClient.ts (+15 -5)
📝 server/routers/client/terminate.ts (+27 -1)
📝 server/routers/newt/handleSocketMessages.ts (+5 -3)
📝 server/routers/olm/deleteUserOlm.ts (+17 -7)
📝 server/routers/olm/handleOlmPingMessage.ts (+72 -77)

📄 Description

Community Contribution License Agreement

By creating this pull request, I grant the project maintainers an unlimited,
perpetual license to use, modify, and redistribute these contributions under any terms they
choose, including both the AGPLv3 and the Fossorial Commercial license terms. I
represent that I have the right to grant this license for all contributed content.

Summary

Fixes #2120 — rising memory floor and CPU spikes that cause Uptime Kuma timeouts and OOM container kills on small instances. Touches the OLM ping hot
path, audit/status log retention, Docker socket cache, Traefik config rebuild cadence, and SQLite driver setup. After this PR the same workload runs
cleanly on a 1 GB instance.

AI Disclosure

This PR was developed with AI assistance. Models used during exploration and implementation: Claude Opus 4.6 and Claude Opus 4.7. Three
different agentic environments were tried — Zed editor (Zed Agent), OpenCode, and Claude Code — with the best results coming from Claude
Code. All changes were reviewed, tested, and validated against a real production workload before merging (see Test Environment).

Test Environment

  • Host: AWS t3a.micro (2 vCPU burstable, 1 GB RAM)
  • OS: Ubuntu 24.04
  • Workload: 2 newt tunnels, 17 public resources, Uptime Kuma actively monitoring the instance
  • Result: 1466+ minutes uptime with stable memory baseline at the time of writing

Changes

1. Throttle expensive OLM ping operations (daafccb1) — biggest impact

File: server/routers/olm/handleOlmPingMessage.ts

Before, every OLM ping ran 5–10 DB operations: full client SELECT, session validation, organization access policy check, config version lookup, and
fingerprint insertion. Under sustained Uptime Kuma probing this was the dominant source of CPU + SQLite contention.

Now the expensive checks run at most once every 5 minutes per OLM. Pings between checks only run the cheap in-memory recordClientPing(), which is
then batch-flushed by the existing ping accumulator. This was the single biggest performance win in the PR.

Known behavioral side-effects. Admin actions (block / delete / unlink) are made immediate via change #2 below. The following state changes are
not push-mitigated and lag by up to ~5 minutes — by design:

  • Natural session expiry / invalidateSession() — OLM keeps tunneling for up to ~7 minutes after the session is invalid (5 min until slow-path
    detection + ~2 min offline-checker grace).
  • Org-membership / access-policy revocation — same ~7 min lag.
  • client.userId mismatch (edge case: client reassigned to a different user) — same lag.
  • Config-version-mismatch fallback sync — primary config push still happens immediately on resource/target updates via incrementConfigVersion: true. Only the ping-side fallback (for missed pushes) is delayed.
  • Fingerprint / posture snapshots — now recorded every 5 min instead of every ping. Affects audit visibility, not active access enforcement.

Other behavior changes worth noting:

  • lastFullCheck Map has no eviction-on-disconnect. ~50 bytes per unique OLM, negligible for typical deployments but technically unbounded over the
    process lifetime. (Eviction was implemented in a follow-up but was bundled with the reverted commit — see Reverted section.)
  • First ping after a process restart triggers a full check for every OLM. Expect a brief CPU + DB burst at startup as many reconnects coincide.
  • Multi-node deployments don't share throttle state — each node has its own lastFullCheck Map.
  • handleFingerprintInsertion() errors are now caught and logged rather than propagated. Stability improvement; behavior change.
  • FULL_CHECK_INTERVAL_MS = 5 * 60 * 1000 is hard-coded — not configurable.

Mitigation paths for deployments that need stricter enforcement:

  • (a) Call disconnectClient(olmId) from your session-invalidation / org-removal paths (deferred follow-up — see terminateAndDisconnect() in
    server/routers/client/terminate.ts for the pattern).
  • (b) Shorten FULL_CHECK_INTERVAL_MS in handleOlmPingMessage.ts, accepting some CPU/memory cost back.

2. Push-based OLM termination on block/delete (07f7151b)

Files: server/routers/client/terminate.ts, blockClient.ts, deleteClient.ts, server/routers/olm/deleteUserOlm.ts

The throttle in change #1 introduced a side-effect: admin actions like blocking or deleting a client took up to ~7 minutes to take effect (5 min
until the next slow-path check + 2 min offline-checker grace). Previously the offline checker handled this via lastPing not being updated, but with
the throttle that mechanism no longer fired on the fast path.

Instead of putting per-ping checks back (see Reverted section), we now push termination from the endpoints that mutate auth state. A new
terminateAndDisconnect(clientId, error, olmId) helper sends the existing olm/terminate message, waits 1 second, then force-closes the WebSocket —
mirroring the pattern already used by server/routers/olm/offlineChecker.ts. Wired into blockClient, deleteClient, and deleteUserOlm.
Termination is moved outside the DB transaction so the 1-second sleep doesn't hold the SQLite write lock.

Admin actions now take effect immediately, with zero per-ping cost.

3. Add statusHistory table cleanup (7001ad8f)

File: server/lib/cleanupLogs.ts

The statusHistory table had no cleanup mechanism and grew without bound, which is one of the contributors to the rising-floor memory pattern. Added
a 90-day retention sweep to the existing 3-hour cleanup interval.

4. Increase TraefikConfigManager default interval 5s → 30s (c561ba51)

File: server/lib/readConfigFile.ts

Each Traefik config rebuild allocates 3–6 MB of temporary objects. At a 5-second interval on a small instance that's significant GC churn for a
config that changes rarely. Default raised to 30s. Existing deployments with monitor_interval set explicitly are unaffected.

5. Add TTL to Docker container/socket cache entries (f03d690f)

File: server/routers/newt/handleSocketMessages.ts

cache.set(..., 0) for socketPath, isAvailable, and dockerContainers meant entries never expired and accumulated permanently as newts churned.
Changed TTL to 300s (5 minutes). The cache is repopulated on demand when the UI fetches container info.

6. Remove autoFinalizeStatement wrapper (16bcc552)

File: server/db/sqlite/driver.ts

The wrapper was added to deterministically finalize prepared statements after each .run() / .get() / .all(), on the assumption that
better-sqlite3 was leaking statements. Investigation showed the assumption is wrong: better-sqlite3's Statement::~Statement() destructor calls
sqlite3_finalize() when the JS wrapper is garbage-collected (see C++ source). The wrapper added per-query overhead, and its
finalize-after-first-execution behavior would have silently broken any reusable prepared statement.

7. Remove mmap_size and cache_size pragmas (1cdd7dba)

File: server/db/sqlite/driver.ts

Both pragmas were causing performance issues on small instances and were removed.

Reverted changes

We tried one approach that had to be reverted. Documenting it because the original reasoning was sound and someone may be tempted to retry.

Reverted: per-ping fast-path block check (ab7c61c4 + 1f8fddf8, reverted in a248a211)

Why we tried it: the throttle in change #1 delayed admin block enforcement by up to 5 minutes. The original commit added a cheap PK SELECT on
clients to every ping so blocked clients would be caught immediately by the offline checker — adding back ~1 DB op per ping in exchange for a
5-minute → 2-minute enforcement window. Query-count math said this was still ~85% below the leaking pre-PR baseline, and the SELECT was projected to
just 4 columns.

Why we reverted: the query-count math missed the actual leak vector. With the autoFinalize wrapper removed (change #6), each per-ping ad-hoc
query allocates a native sqlite3_stmt that only releases on V8 GC. Drizzle doesn't cache statements automatically. On a 1 GB instance under
sustained ping load, GC fell behind native allocation, off-heap RSS climbed, and the symptoms reappeared (Uptime Kuma timeouts, OOM kills) within
hours of deploying.

The proper fix turned out to be push-based termination (change #2), which has zero per-ping cost and gives faster enforcement than the per-ping check
would have.

Lesson: in this codebase, any per-ping ad-hoc DB query in the OLM ping path is suspect on small instances. If per-ping checks are ever genuinely
needed, they must use explicitly cached prepared statements (drizzle.prepare()) or in-memory state with explicit invalidation — not ad-hoc queries.

Operational notes

WAL mode and HTTPS Request Log Retention

WAL mode is opt-in via ENABLE_SQLITE_WAL_MODE=true (see c8e7e0ee).

Empirically:

  • WAL off (default): stable on a 1 GB instance with HTTPS request log retention disabled.
  • WAL off + HTTPS request log retention enabled: write-lock contention from per-request audit log inserts cascades into event-loop stalls and
    forced restarts on small instances.
  • WAL on + HTTPS request log retention enabled: stable.

Recommendation for ≤ 1 GB deployments: if you want HTTPS request log retention turned on, set ENABLE_SQLITE_WAL_MODE=true. With request logging
off, the default (WAL off) is fine.

WAL remains opt-in rather than default because the disable in c8e7e0ee was deliberate — see that commit for the rationale.

Test plan

  • OLM tunnels stay connected and continue to ping under load
  • Admin block / delete / unlink endpoints terminate active OLM sessions immediately
  • statusHistory retention sweep runs without errors
  • Docker container/socket cache repopulates on demand after TTL expiry
  • Stable memory baseline over 24+ hours on t3a.micro (verified 1466+ min)
  • Stable behavior with HTTPS Request Log Retention enabled when ENABLE_SQLITE_WAL_MODE=true

🔄 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/3138 **Author:** [@Josh-Voyles](https://github.com/Josh-Voyles) **Created:** 5/23/2026 **Status:** ❌ Closed **Base:** `dev` ← **Head:** `mem-tune-2-oc` --- ### 📝 Commits (10+) - [`1cdd7db`](https://github.com/fosrl/pangolin/commit/1cdd7dba21d2eb535dad54c6b8510d2cb2c9cc45) remove mmap_size n cache_size - perf issues small instances - [`daafccb`](https://github.com/fosrl/pangolin/commit/daafccb1a89849b1a2702eca7568453322e8f616) fix: throttle expensive OLM ping operations to once per 5 minutes (#2120) - [`7001ad8`](https://github.com/fosrl/pangolin/commit/7001ad8f5a0ab586dcf5eab538811a9377998ed4) fix: add statusHistory table cleanup with 90-day retention (#2120) - [`c561ba5`](https://github.com/fosrl/pangolin/commit/c561ba51da00798254093f7786cc042c97bf84ce) fix: increase TraefikConfigManager default interval to 30s (#2120) - [`f03d690`](https://github.com/fosrl/pangolin/commit/f03d690f138037c7561a9d31284d3c7e784bd996) fix: add TTL to Docker container cache entries (#2120) - [`16bcc55`](https://github.com/fosrl/pangolin/commit/16bcc55205e5ccc577d6fcab07e1b99739a399b7) fix: remove unnecessary autoFinalizeStatement wrapper (#2120) - [`ab7c61c`](https://github.com/fosrl/pangolin/commit/ab7c61c46fcc22ceeb493671b53ae6f6c33cb0fa) fix: restore immediate block enforcement on OLM ping path (#2120) - [`1f8fddf`](https://github.com/fosrl/pangolin/commit/1f8fddf8cb93fb8eae62041294eaeea92b19df9e) fix: select full client row for sendOlmSyncMessage (#2120) - [`a248a21`](https://github.com/fosrl/pangolin/commit/a248a211583b46e328d13df18171ae58883deb7c) revert: per-ping block check on OLM ping path (#2120) - [`07f7151`](https://github.com/fosrl/pangolin/commit/07f7151bd1b2a015dd95c30b13840761672b0d65) feat: push-based OLM termination on block/delete (#2120) ### 📊 Changes **9 files changed** (+160 additions, -152 deletions) <details> <summary>View changed files</summary> 📝 `server/db/sqlite/driver.ts` (+2 -48) 📝 `server/lib/cleanupLogs.ts` (+10 -3) 📝 `server/lib/readConfigFile.ts` (+1 -1) 📝 `server/routers/client/blockClient.ts` (+11 -7) 📝 `server/routers/client/deleteClient.ts` (+15 -5) 📝 `server/routers/client/terminate.ts` (+27 -1) 📝 `server/routers/newt/handleSocketMessages.ts` (+5 -3) 📝 `server/routers/olm/deleteUserOlm.ts` (+17 -7) 📝 `server/routers/olm/handleOlmPingMessage.ts` (+72 -77) </details> ### 📄 Description ## Community Contribution License Agreement By creating this pull request, I grant the project maintainers an unlimited, perpetual license to use, modify, and redistribute these contributions under any terms they choose, including both the AGPLv3 and the Fossorial Commercial license terms. I represent that I have the right to grant this license for all contributed content. ## Summary Fixes #2120 — rising memory floor and CPU spikes that cause Uptime Kuma timeouts and OOM container kills on small instances. Touches the OLM ping hot path, audit/status log retention, Docker socket cache, Traefik config rebuild cadence, and SQLite driver setup. After this PR the same workload runs cleanly on a 1 GB instance. ## AI Disclosure This PR was developed with AI assistance. Models used during exploration and implementation: **Claude Opus 4.6** and **Claude Opus 4.7**. Three different agentic environments were tried — **Zed editor (Zed Agent)**, **OpenCode**, and **Claude Code** — with the best results coming from Claude Code. All changes were reviewed, tested, and validated against a real production workload before merging (see Test Environment). ## Test Environment - **Host:** AWS `t3a.micro` (2 vCPU burstable, 1 GB RAM) - **OS:** Ubuntu 24.04 - **Workload:** 2 newt tunnels, 17 public resources, Uptime Kuma actively monitoring the instance - **Result:** 1466+ minutes uptime with stable memory baseline at the time of writing ## Changes ### 1. Throttle expensive OLM ping operations (`daafccb1`) — biggest impact **File:** `server/routers/olm/handleOlmPingMessage.ts` Before, every OLM ping ran 5–10 DB operations: full client SELECT, session validation, organization access policy check, config version lookup, and fingerprint insertion. Under sustained Uptime Kuma probing this was the dominant source of CPU + SQLite contention. Now the expensive checks run at most once every 5 minutes per OLM. Pings between checks only run the cheap in-memory `recordClientPing()`, which is then batch-flushed by the existing ping accumulator. This was the single biggest performance win in the PR. **Known behavioral side-effects.** Admin actions (block / delete / unlink) are made immediate via change #2 below. The following state changes are *not* push-mitigated and lag by up to ~5 minutes — by design: - **Natural session expiry / `invalidateSession()`** — OLM keeps tunneling for up to ~7 minutes after the session is invalid (5 min until slow-path detection + ~2 min offline-checker grace). - **Org-membership / access-policy revocation** — same ~7 min lag. - **`client.userId` mismatch** (edge case: client reassigned to a different user) — same lag. - **Config-version-mismatch fallback sync** — primary config push still happens immediately on resource/target updates via `incrementConfigVersion: true`. Only the ping-side fallback (for missed pushes) is delayed. - **Fingerprint / posture snapshots** — now recorded every 5 min instead of every ping. Affects audit visibility, not active access enforcement. **Other behavior changes worth noting:** - `lastFullCheck` Map has no eviction-on-disconnect. ~50 bytes per unique OLM, negligible for typical deployments but technically unbounded over the process lifetime. (Eviction was implemented in a follow-up but was bundled with the reverted commit — see Reverted section.) - First ping after a process restart triggers a full check for every OLM. Expect a brief CPU + DB burst at startup as many reconnects coincide. - Multi-node deployments don't share throttle state — each node has its own `lastFullCheck` Map. - `handleFingerprintInsertion()` errors are now caught and logged rather than propagated. Stability improvement; behavior change. - `FULL_CHECK_INTERVAL_MS = 5 * 60 * 1000` is hard-coded — not configurable. **Mitigation paths for deployments that need stricter enforcement:** - (a) Call `disconnectClient(olmId)` from your session-invalidation / org-removal paths (deferred follow-up — see `terminateAndDisconnect()` in `server/routers/client/terminate.ts` for the pattern). - (b) Shorten `FULL_CHECK_INTERVAL_MS` in `handleOlmPingMessage.ts`, accepting some CPU/memory cost back. ### 2. Push-based OLM termination on block/delete (`07f7151b`) **Files:** `server/routers/client/terminate.ts`, `blockClient.ts`, `deleteClient.ts`, `server/routers/olm/deleteUserOlm.ts` The throttle in change #1 introduced a side-effect: admin actions like blocking or deleting a client took up to ~7 minutes to take effect (5 min until the next slow-path check + 2 min offline-checker grace). Previously the offline checker handled this via `lastPing` not being updated, but with the throttle that mechanism no longer fired on the fast path. Instead of putting per-ping checks back (see Reverted section), we now push termination from the endpoints that mutate auth state. A new `terminateAndDisconnect(clientId, error, olmId)` helper sends the existing `olm/terminate` message, waits 1 second, then force-closes the WebSocket — mirroring the pattern already used by `server/routers/olm/offlineChecker.ts`. Wired into `blockClient`, `deleteClient`, and `deleteUserOlm`. Termination is moved outside the DB transaction so the 1-second sleep doesn't hold the SQLite write lock. Admin actions now take effect immediately, with zero per-ping cost. ### 3. Add `statusHistory` table cleanup (`7001ad8f`) **File:** `server/lib/cleanupLogs.ts` The `statusHistory` table had no cleanup mechanism and grew without bound, which is one of the contributors to the rising-floor memory pattern. Added a 90-day retention sweep to the existing 3-hour cleanup interval. ### 4. Increase TraefikConfigManager default interval 5s → 30s (`c561ba51`) **File:** `server/lib/readConfigFile.ts` Each Traefik config rebuild allocates 3–6 MB of temporary objects. At a 5-second interval on a small instance that's significant GC churn for a config that changes rarely. Default raised to 30s. Existing deployments with `monitor_interval` set explicitly are unaffected. ### 5. Add TTL to Docker container/socket cache entries (`f03d690f`) **File:** `server/routers/newt/handleSocketMessages.ts` `cache.set(..., 0)` for `socketPath`, `isAvailable`, and `dockerContainers` meant entries never expired and accumulated permanently as newts churned. Changed TTL to 300s (5 minutes). The cache is repopulated on demand when the UI fetches container info. ### 6. Remove `autoFinalizeStatement` wrapper (`16bcc552`) **File:** `server/db/sqlite/driver.ts` The wrapper was added to deterministically finalize prepared statements after each `.run() / .get() / .all()`, on the assumption that `better-sqlite3` was leaking statements. Investigation showed the assumption is wrong: `better-sqlite3`'s `Statement::~Statement()` destructor calls `sqlite3_finalize()` when the JS wrapper is garbage-collected (see C++ source). The wrapper added per-query overhead, and its finalize-after-first-execution behavior would have silently broken any reusable prepared statement. ### 7. Remove `mmap_size` and `cache_size` pragmas (`1cdd7dba`) **File:** `server/db/sqlite/driver.ts` Both pragmas were causing performance issues on small instances and were removed. ## Reverted changes We tried one approach that had to be reverted. Documenting it because the original reasoning was sound and someone may be tempted to retry. ### Reverted: per-ping fast-path block check (`ab7c61c4` + `1f8fddf8`, reverted in `a248a211`) **Why we tried it:** the throttle in change #1 delayed admin block enforcement by up to 5 minutes. The original commit added a cheap PK `SELECT` on `clients` to every ping so blocked clients would be caught immediately by the offline checker — adding back ~1 DB op per ping in exchange for a 5-minute → 2-minute enforcement window. Query-count math said this was still ~85% below the leaking pre-PR baseline, and the SELECT was projected to just 4 columns. **Why we reverted:** the query-count math missed the actual leak vector. With the `autoFinalize` wrapper removed (change #6), each per-ping ad-hoc query allocates a native `sqlite3_stmt` that only releases on V8 GC. Drizzle doesn't cache statements automatically. On a 1 GB instance under sustained ping load, GC fell behind native allocation, off-heap RSS climbed, and the symptoms reappeared (Uptime Kuma timeouts, OOM kills) within hours of deploying. The proper fix turned out to be push-based termination (change #2), which has zero per-ping cost and gives faster enforcement than the per-ping check would have. **Lesson:** in this codebase, any per-ping ad-hoc DB query in the OLM ping path is suspect on small instances. If per-ping checks are ever genuinely needed, they must use explicitly cached prepared statements (`drizzle.prepare()`) or in-memory state with explicit invalidation — not ad-hoc queries. ## Operational notes ### WAL mode and HTTPS Request Log Retention WAL mode is opt-in via `ENABLE_SQLITE_WAL_MODE=true` (see `c8e7e0ee`). Empirically: - **WAL off** (default): stable on a 1 GB instance with HTTPS request log retention **disabled**. - **WAL off + HTTPS request log retention enabled**: write-lock contention from per-request audit log inserts cascades into event-loop stalls and forced restarts on small instances. - **WAL on + HTTPS request log retention enabled**: stable. **Recommendation for ≤ 1 GB deployments:** if you want HTTPS request log retention turned on, set `ENABLE_SQLITE_WAL_MODE=true`. With request logging off, the default (WAL off) is fine. WAL remains opt-in rather than default because the disable in `c8e7e0ee` was deliberate — see that commit for the rationale. ## Test plan - [x] OLM tunnels stay connected and continue to ping under load - [x] Admin block / delete / unlink endpoints terminate active OLM sessions immediately - [x] `statusHistory` retention sweep runs without errors - [x] Docker container/socket cache repopulates on demand after TTL expiry - [x] Stable memory baseline over 24+ hours on `t3a.micro` (verified 1466+ min) - [x] Stable behavior with HTTPS Request Log Retention enabled when `ENABLE_SQLITE_WAL_MODE=true` --- <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-05-29 02:53:52 -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#21473
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?