flyteorg / flyteorg/flyte

[flyte2] Executor caches every Event cluster-wide and never evicts objectCache buckets

Open
#7,992 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
7.5k
Forks
886
Avg merge
1d 14h
Merged PRs (30d)
120

Description

## Describe the bug

The executor's object-event watcher registers an informer for `events.k8s.io/v1 Event` that is **not scoped**, and the cache it fills **never releases entries**. Both reintroduce the failure mode that #7831 reported and #7837 fixed — but for Events rather than Pods, so the earlier fix does not cover it.

There are two compounding problems.

### 1. The Event informer is cluster-wide and unfiltered

`executor.Setup` restricts the manager cache for exactly one type:

https://github.com/flyteorg/flyte/blob/6c14e255170c89f2893a096702f72bfd482bc010/executor/setup.go#L134-L142

```go
cacheOptions := cache.Options{
ByObject: map[client.Object]cache.ByObject{
&corev1.Pod{}: {
Label: labels.SelectorFromSet(labels.Set{
flytek8s.ManagedLabelKey: flytek8s.ManagedLabelValue,
}),
},
},
}
```

The comment directly above it states the rationale:

> controller-runtime caches every watched object across all namespaces, so the manager would otherwise hold every Pod in the cluster. On a large multi-tenant cluster that costs several GB of resident memory and OOMKills the executor.

That reasoning applies just as much to Events, but there is no `ByObject` entry for them. The event watcher then asks the same cache for an Event informer:

https://github.com/flyteorg/flyte/blob/6c14e255170c89f2893a096702f72bfd482bc010/executor/pkg/plugin/k8s/event_watcher.go#L52-L56

```go
informer, err := cache.GetInformer(ctx, &eventsv1.Event{})
```

In controller-runtime v0.24.1 (the pinned version), `ByObject` builds a `delegatingByGVKCache` with a per-GVK cache only for the listed types; every other GVK falls through to `defaultCache`, constructed via `optionDefaultsToConfig(&opts)` over `corev1.NamespaceAll`. Since `DefaultLabelSelector`, `DefaultFieldSelector` and `DefaultNamespaces` are all unset here, the Event informer does a **full, unfiltered LIST/WATCH of every Event in every namespace**. The shipped ClusterRole grants `list`/`watch` on `events`/`events.k8s.io` cluster-wide, so this succeeds rather than failing closed.

Events are typically the highest-churn object in a cluster, and the executor caches all of them — including events regarding Deployments, Nodes, Jobs, CronJobs and every other object that has nothing to do with Flyte.

This is not opt-in. It runs at startup for every registered k8s plugin, with no config gate:

https://github.com/flyteorg/flyte/blob/6c14e255170c89f2893a096702f72bfd482bc010/executor/pkg/plugin/registry.go#L68

### 2. `objectCache` grows monotonically and never evicts

`store` creates one bucket per `(namespace, name, kind)` of the object each event regards:

https://github.com/flyteorg/flyte/blob/6c14e255170c89f2893a096702f72bfd482bc010/executor/pkg/plugin/k8s/event_watcher.go#L97-L100

`OnDelete` removes the individual event from the inner map, but deliberately leaves the bucket behind:

https://github.com/flyteorg/flyte/blob/6c14e255170c89f2893a096702f72bfd482bc010/executor/pkg/plugin/k8s/event_watcher.go#L180-L183

```go
delete(eventInfos.eventInfos, eventKey)
// We intentionally do not delete empty buckets from objectCache. This avoids races where
// a new event is being added to the bucket while the top-level map entry is concurrently removed.
```

The race that comment avoids is real, but the consequence is that there is **no eviction path whatsoever** — `objectCache` has no `Delete`, `LoadAndDelete` or `Range`-based reaper anywhere in the tree. Kubernetes expires Events after ~1h, so the inner maps drain, but the outer buckets accumulate for the entire process lifetime.

Because the informer is cluster-wide (problem 1), the key space is not "Flyte task pods" — it is *every object in the cluster that has ever emitted an event since the executor started*. And since pod names are unique per attempt (`buildGeneratedName` → `{action}-{retry}`), even the Flyte-only subset never reaches a steady state; it grows with cumulative attempts, not with concurrency.

## Measurement

Driving `store` and `OnDelete` directly for 200k distinct objects, then deleting every event and forcing a GC:

```
distinct objects seen: 200000
buckets retained in objectCache AFTER all events deleted: 200000
event entries retained inside buckets: 0
retained heap delta: 115.9 MB (~608 bytes per retained bucket)
```

Every event was deleted and every inner map is empty, yet all 200k buckets and ~116 MB are still held. That is ~608 bytes retained permanently per distinct object the cluster emits an event about. This measures only problem 2 — the retained `objectCache` — and excludes the informer's own copy of the Event objects from problem 1, which is the larger of the two.

## Expected behavior

The executor's memory footprint should scale with its own workload, not with the size and event volume of the cluster it happens to run on — the same expectation #7831 established for Pods.

## Suggested direction

1. **Scope the Event informer.** Add an `&eventsv1.Event{}` entry to `cacheOptions.ByObject`. A field selector on `regarding.kind=Pod` plus the namespace scoping already available would cut most of it; `DefaultTransform`/`TransformStripManagedFields` would shrink what is retained further. Events cannot carry the Flyte managed label, so a label selector is not available the way it was for Pods.
2. **Bound `objectCache`.** Options that keep the documented race closed: drop buckets whose inner map is empty under the bucket's own write lock while re-checking emptiness, or attach a TTL/`Range` reaper, or bound it with an LRU. Since the watcher only ever serves lookups for the pod of a live TaskAction, evicting a bucket on task terminal transition would also work and is the tightest fit.
3. Consider whether the watcher should be gated behind config at all, given it is currently unconditional and only consumers of GPU-fault classification and `AdditionalReasons` need it.

## Additional context

- Reproduced against `main` @ `6c14e255170c89f2893a096702f72bfd482bc010` (current release v2.0.48).
- `event_watcher.go` predates the Pod fix — it landed in the v2 cutover (#6583, 2026-04-28), while the Pod scoping landed later (#7837, 2026-08-14) and covered only `corev1.Pod`.
- Related but distinct: #7831 (Pods, fixed by #7837). This is the same class of bug for a type that fix did not cover, plus the separate unbounded-growth problem in `objectCache`.

Contributor guide

Open the contributing guide

Research direction

Start in executor/setup.go and executor/pkg/plugin/k8s/event_watcher.go, then inspect executor/pkg/plugin/registry.go to understand when the watcher is created. Reproduce the store and OnDelete behavior with distinct objects, and verify that event watching is scoped and empty objectCache buckets do not remain retained. Done means executor memory no longer grows with unrelated cluster Events or cumulative completed objects.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, kubernetes
Domain
backend, infrastructure
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.