chore: Optimize event controller cache hydration for high-volume clusters #124
Conversation
engedaam
changed the title
engedaam
force-pushed
the
use-clientgo-watcher
branch
2 times, most recently
from
a882103 to
ce1c5cc
Compare
engedaam
changed the title
engedaam
changed the title
engedaam
changed the title
|
Small nit just about commit cleanliness: You should maybe consider squashing commits so you don't end-up with a 22-commit PR -- doesn't really matter since we end-up squashing commits in the PR anyways, but something to note for other repos that might use rebase merges |
engedaam
force-pushed
the
use-clientgo-watcher
branch
from
7b05b23 to
6e47626
Compare
engedaam
force-pushed
the
use-clientgo-watcher
branch
from
3e6e5c0 to
56c5c39
Compare
| client := fake.NewSimpleClientset() | ||
| eventChannel = make(chan watch.Event, 1000) | ||
| controller = events.NewController[*test.CustomObject](ctx, kubeClient, fakeClock, client) | ||
| controller.EventWatchChannel = eventChannel |
| } | ||
|
|
||
| func (c *Controller[T]) Reconcile(ctx context.Context, event *v1.Event) (reconcile.Result, error) { | ||
| func (c *Controller[T]) Reconcile(ctx context.Context) (reconcile.Result, error) { | ||
| e := <-c.EventWatchChannel |
There was a problem hiding this comment.
I just took a look at the underlying code and the implementation of the ResultChan uses an unbuffered channel -- that means new writes are going to get blocked on reads here -- I'm concerned that we may block the watch stream if we don't either pull these events into a buffered channel OR use multiple goroutines to process these events.
I'm trying to think of a good way to handle this while keeping the same interface so that we keep the same logging behavior and metrics -- nothing comes to mind immediately, but I think it's something that we should explore. Minimally, we can kick off a goroutine in the register that pulls data off of the watch channel and puts it in a separate buffered channel -- another option is to keep a static cache of events as we see them, enqueue a reconcile.Request and then read from that local cache (this is basically exactly what controller-runtime does with a read cache without the deletions)
jonathan-innis
dismissed
their stale review
Accidentally submitted approval -- meant to just submit comments
*Issue N/A
Problem:
When dealing with clusters that generate high volumes of events, the initial cache hydration in controller-runtime creates significant API Server load. This occurs because controller-runtime performs a LIST operation on all historical events during startup.
Solution:
Since we only care about events that occur after our controller starts, we're optimizing this by:
Benefits:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.