Sign up
View stargazersSign up

[Bug]: Server memory grows ~1.7 GB/hour: every one-minute pull-request sync sweep stays in memory#25172

opened4d agobyBbompus

Before submitting

  • I searched existing issues and did not find a duplicate.
  • I included enough detail to reproduce or investigate the problem.

Area

apps/server

Steps to reproduce

  1. Run the nightly server (t3 serve) with many threads that have linked pull requests, with NODE_OPTIONS=--heapsnapshot-signal=SIGUSR2 set. Mine has 2,866 threads, 950 of them with links, 18,736 links in total, 4,035 distinct pull requests and a 20 GB statev2.sqlite. I have not tried a small data set, so I do not know the smallest size that shows this.
  2. Note the server's RSS every minute.
  3. At about 10 and 30 minutes, run kill -USR2 <t3 serve pid> to write a heap snapshot.
  4. In each snapshot, count the Map objects held by a closure variable named groups.

Result on 0.1.3-nightly.20261025.3692 after a fresh restart: 11 such Maps at 11.7 minutes of uptime, 29 at 31.8 minutes.

Expected behavior

Each one-minute pull-request sync sweep (PullRequestSyncReactor.sweep) builds a groups map from every thread that has linked pull requests. When the sweep ends, that map and the thread and link data under it should become garbage, so memory stays flat.

Actual behavior

Every sweep's groups map and all the data under it stays reachable from the GC roots, about one more copy per minute.

  • 0.1.3-nightly.20261025.3692, fresh restart: 11 retained Maps at 11.7 min, 29 at 31.8 min. Nodes reachable from the GC roots went from 5.3M to 10.7M. RSS rose from 667 MB to 870 MB in the first 8 minutes (about 25 MB per minute), before any snapshot was taken.
  • 0.1.3-nightly.20261025.3669, after 3 hours: RSS was 970 MiB 30 seconds after start and 6.2 GB at 3 hours. The heap snapshot at that point held 215 retained Maps and about 2.8 million link-snapshot objects, roughly 151 copies of the 18,736 links in the database.

The retention paths run through Effect fiber stack frames that are kept alive by an entry in the server's RPC read cache (variables named readCache and revisions, which looks like apps/server/src/pullRequest/PullRequestReadCache.ts; I did not confirm the exact line). One path goes through the delayed-effect pool instead, and one through an Error object's callError context. Trimmed paths are under "Logs".

At this data size the default 4 GB heap runs out after about 2.4 hours, which is what happened here. A 16 GB cap (--max-old-space-size=16384) lasts about 9 hours. I am not claiming the leak alone made the server unresponsive: it became very slow only after I wrote a heap snapshot of a 6 GB heap, which pushed RSS to 33 GB.

Related but different: #24098, #18618 and #24797 describe CPU and git work from the same sweeps. This report is about memory that is never released.

Impact

Major degradation or frequent failure

Version or commit

0.1.3-nightly.20261025.3692 (also seen on 0.1.3-nightly.20261025.3669)

Environment

Linux x64, headless t3 serve run as a systemd user service, NODE_OPTIONS=--max-old-space-size=16384

Logs or stack traces

shell
1Retention paths to a retained `groups` Map in the 0.1.3-nightly.20261025.3669 snapshot, GC root first (trimmed; "x5" means five repeats):
2
3path 1:
4(GC roots) > (Global handles) > process > closure(cwd) > Context > Context(previous) > Array [context var serverWsRpcs] > element 30: closure Rpc > Object [key Rpc -> value] > property context > Map (base) > table > Object > closure list > Context > Object [context var readCache] > closure get > Context > closure body > Context > Object [context var cache] > Object inMemory > Object map > Map backing > table > EntryImpl > property fiber: FiberImpl > property cache: Object > property stackFrame: Object > property parent (x5) > closure [property stack] > Context > closure [context stack] > Context > closure [context body] > Context > Map [context groups]
5
6path 2:
7(GC roots) > (Global handles) > process > closure(cwd) > Context > Context(previous) > Array [context var batchPool] > element 4: Object > ContImpl [property delayEffect] > closure [property payload] > Context > Context(previous) > closure [context resume] > Context > FiberImpl [context fiber] > Object [property cache] > property stackFrame: Object > property parent (x5) > closure [property stack] > Context > closure [context stack] > Context > closure [context body] > Context > Map [context groups]
8
9path 3 ends: ... > Error [context callError] > Array [property <symbol>] > closure > Context > Map [context groups]
10
11Heap snapshot of the 3669 server at 3 h: 67.4M nodes. Strings 2.0 GB (40.6M), plain Object 774 MB (15.3M), property arrays 483 MB (5.8M). The retained link objects have these property names, each about 2.8M times: repository, host, name, state, title, url, headBranch, isDraft, author, baseBranch, mergeability, snapshot, link, thread, pullRequests, syncedAt, linkedAt. RSS: 970 MiB at 30 s, 6.2 GB at 3 h.

Screenshots, recordings, or supporting files

No response

Workaround

Restarting the service (t3 service restart) clears it.

Bbompus3d ago
Synced from GitHub

Finished sweeps are still retained after the read-cache fix. I ran the same change as #25285 (detachStackFrame around the read-cache lookups) on a server with about 18,700 thread pull request links and took heap snapshots at 11 minutes and at about 3 hours of uptime. Growth dropped from 22-25 MiB/min without the change to about 1 MiB/min, but old sweeps are still held:

after 11 minafter 3 h
groups maps from finished sweeps still reachable14
objects carrying a pull request avatarUrl14,12257,528
live JS heap337 MiB508 MiB

(The two snapshots come from different nightlies, 3692 and 3717, both with the change, so the heap figures are indicative only. Whether the map count keeps rising past 3 h is not measured.)

The mechanism is the same as in this issue: a Cache entry keeps the fiber that filled it, and that fiber keeps the caller's lazy stack frames, which close over the sweep. In the 3 h snapshot the cache entries that still reach a retained map are 13 in repositoryRootCache and 11 in repositoryIdentityCache (RepositoryIdentityResolver.resolve, positive TTL 15 min), plus a few through detectionCache (VcsDriverRegistry.detect, which the identity lookup calls). The first sweep to look a directory up fills the entry, and later sweeps read it without replacing it, so that first sweep stays alive.

Wrapping those four lookups in detachStackFrame is a few-line change. A test that runs five sweeps through resolve and checks that none of their data survives garbage collection fails without it and passes with it. I have not yet measured the effect on a long-running server. If it is useful I can open a pull request once this issue is triaged, and post the 3 h snapshot result here first.

Edited after review: an earlier version of this comment also named detailCache (PullRequestService.detail) and counted five lookups. The sync sweep does not call detail, so it is not part of the change.

Bbompus3d ago
Synced from GitHub

@juliusmarminge a note on how this relates to #25177. After #25285 merged, I ran the same read-cache change on a server with about 18,700 thread pull request links. A heap snapshot at about 3 hours still held four finished sweeps' groups maps, against one at 11 minutes (details in my comment above). They are held through Cache entries in repositoryRootCache and repositoryIdentityCache (the lookups in RepositoryIdentityResolver.resolve), whose fibers keep the calling sweep's lazy stack frames.

From its description, #25177 removes that at the source by making the two helpers inside each sweep stop capturing the sweep. Do you expect it to cover these two caches as well, or should those lookups be wrapped in detachStackFrame, as #25285 did for the read cache? I have a small change with a test for that and have not opened a pull request for it.

I'll post a 3 h snapshot result here once I have run the next nightly with the fix applied.

Bbompus3d ago
Synced from GitHub

Result with #25177 applied: no finished sweeps are retained after about 3 hours.

I built nightly 20261026.3740 (which includes #25285) with #25177 applied on top (head 57a7bd0a8d, nothing else) and ran it on the same server, with about 18,700 thread pull request links. I took a heap snapshot at 2 h 55 min of uptime and compared it with the 3 h snapshot from the build I described above (nightly 3717 with only the detachStackFrame read-cache change):

at about 3 h3717 + detachStackFrame read-cache3740 + #25177
groups maps from finished sweeps still reachable40
objects carrying a pull request avatarUrl57,5285,800
live JS heap508 MiB453 MiB
cache entries (repositoryRootCache, repositoryIdentityCache) reaching a retained map240

So #25177 also releases what the repository root and identity caches were holding, and the extra wrappers I suggested are not needed on top of it.

Caveats: one snapshot per build, a few minutes short of 3 h, and the two builds differ by more than this change (3717 vs 3740), so the heap figure is indicative. The maps and cache-entry counts are exact counts from the snapshots. I have not looked past 3 h.