Skip to content

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

Description

@Daksha1611

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:

flyte/executor/setup.go

Lines 134 to 142 in 6c14e25

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

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:

func newControllerRuntimeEventWatcher(ctx context.Context, cache ctrlcache.Cache) (*controllerRuntimeEventWatcher, error) {
informer, err := cache.GetInformer(ctx, &eventsv1.Event{})
if err != nil {
return nil, err
}

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:

if err := pm.InitializeObjectEventWatcher(ctx); err != nil {

2. objectCache grows monotonically and never evicts

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

value, _ := w.objectCache.LoadOrStore(objectKey, &eventObjects{
eventInfos: make(map[k8stypes.NamespacedName]*eventInfo),
})
eventInfos := value.(*eventObjects)

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

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.
}

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 whatsoeverobjectCache 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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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