Skip to content

In-memory instance cache leaks deleted instances, causing runner scale-up starvation #847

Description

@benoit-nexthop

Summary

The in-memory instance cache (cache/instance_cache.go) accumulates stale entries for instances that were long ago deleted from the database. Over ~9 days of normal operation our GARM instance's cache grew to ~900 instances while the database contained only 77, of which 819 cached entries were tombstones with status: deleted. Anything served from the cache (e.g. the /api/v1/ws/metrics dashboard snapshot, per-pool/scale-set runner counts, etc.) reports wildly inflated numbers. Restarting GARM "fixes" it (cache reloads from the DB), and the leak then starts again.

Environment

  • GARM main (reproduced against current main, commit 2edcc2ff)
  • 6 scale sets (~60–80 concurrent runners, frequent scale up/down churn)

Observed behavior

Dashboard reported:

Instances: 895   (16 active / 329 idle / 53 pending / 497 offline)

while GET /api/v1/instances returned 77 instances, all running. Dumping a snapshot from /api/v1/ws/metrics shows the cache contents:

total instances in snapshot: 896
by instance status: deleted: 819, running: 74, deleting: 2, pending_delete: 1
by runner status:   terminated: 494, idle: 328, active: 16, pending: 48, installing: 5, offline: 5

819 cache entries have status: deleted — the terminal status set just before the DB row is removed — and they are never evicted.

Root cause

Instance deletion emits two watcher events in sequence:

  1. UpdateOperation with Status = InstanceDeleted (set by the provider worker via SetInstanceStatus)
  2. DeleteOperation when the row is removed (DeleteInstanceByName / DeleteInstance)

The cache worker (workers/cache/cache.go, handleInstanceEvent) stores the instance on create/update and evicts it on delete. This relies on events being delivered in order and reliably. The watcher guarantees neither:

1. Events are delivered out of order

database/watcher/watcher.go, serviceProducer:

case payload := <-prod.messages:
    w.mux.Lock()
    for _, c := range w.consumers {
        go c.Send(payload)   // one goroutine per event per consumer
    }
    w.mux.Unlock()

Each event is handed to each consumer in a fresh goroutine. Goroutine scheduling order is unspecified, so two events emitted back-to-back can be enqueued to a consumer in reverse order. When the DeleteOperation is processed before the UpdateOperation(status=deleted), the update re-inserts the already-deleted instance into the cache. Since InstanceDeleted is a terminal state, no further event ever evicts it — the entry leaks until process restart.

This matches what we observed: every leaked entry had status: deleted.

2. Events are silently dropped under load

database/watcher/consumer.go, Send:

timer := time.NewTimer(1 * time.Second)
...
select {
...
case <-timer.C:
    slog.DebugContext(w.ctx, "timeout trying to send payload", "payload", payload)  // event lost
case w.messages <- payload:
}

Consumer channels are buffered at 1 (messages: make(chan common.ChangePayload, 1)). During deletion bursts (scale-down of many runners), sends time out after 1s and the event is dropped with only a debug log. A dropped DeleteOperation leaks the cache entry the same way.

There is no periodic reconciliation of the cache against the database — it is populated once at startup and then maintained purely from this lossy, unordered event stream, so any missed/reordered eviction persists forever.

Impact

  • GARM's built-in dashboard instance counts and runner-status breakdowns drift ever upward (ours was off by >10x after 9 days).
  • Any consumer of cache.GetAllInstancesCache() / GetInstancesForPool / GetInstancesForScaleSet sees phantom instances; other watcher consumers (scale set workers, etc.) are exposed to the same reordering/drop hazards.

Secondary effect: scale-up starvation (jobs wait many minutes for a runner)

The scale-set autoscaler is driven by the worker-local w.runners map (workers/scaleset/scaleset.go), which is maintained from the same unordered/lossy watcher stream. handleAutoScale compares len(w.runners) against min(min_idle_runners + desired_runner_count, max_runners) and only creates runners when below target.

Two ways stale entries inflate that count:

  1. Reordered events: a late UpdateOperation (status running/idle) processed after the DeleteOperation re-inserts a dead runner. The periodic cleanup only evicts entries with status deleted, so it stays.
  2. Zero-value re-insertion: when consolidation finds a w.runners entry missing from GitHub, it calls setRunnerDBStatus then UpdateInstance. If the DB row is already gone, ErrNotFound is swallowed and a zero-value params.Instance{} is returned with nil error, then stored back via w.runners[runner.ID] = instance. An entry with Status: "" matches no cleanup path and permanently occupies a runner slot. The same pattern exists in removeRunnerFromGithubAndSetPendingDelete.

Each stale entry makes the worker believe it is closer to (or above) target, so it refuses to scale up — and may scale down real idle runners. We observed queued jobs waiting 10-20 minutes for a runner during a near-idle weekend while the cache held 389 entries for a scale set with max_runners: 100 (actual DB instances: 45). Restarting GARM resolved this too.

Suggested additional fix: in setRunnerDBStatus callers, do not store the returned instance when UpdateInstance returned ErrNotFound — evict the entry from w.runners instead.

Workaround

Restart GARM so that the cache is rebuilt from the DB and counts are correct again until the leak re-accumulates.

Suggested fixes

Any of these (ideally the first two together):

  1. Preserve per-consumer ordering: replace go c.Send(payload) with a per-consumer serialized queue (single dispatcher goroutine per consumer draining a larger buffer), so events arrive in emission order.
  2. Defense in depth in the cache worker: treat UpdateOperation with Status == InstanceDeleted as an eviction rather than an upsert.
  3. Periodic reconciliation: have the cache worker periodically re-list instances from the DB and evict cache entries that no longer exist, so any missed event self-heals.
  4. Increase consumer channel buffers and/or surface send timeouts at warn/error level so drops are visible.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions