Reduce memory usage of watcher

[henrikno]

According to topfreegames/maestro#473
kubernetes/kubernetes#103789
https://github.com/kubernetes/sample-controller/blob/master/docs/controller-client-go.md
https://github.com/kubernetes/client-go/blob/master/examples/workqueue/main.go
we're supposed to only put the key on the work queue, and retrieve it from the indexer when processing it.
But in

elastic-agent-autodiscover/kubernetes/watcher.go

Line 243 in e092f62

w.queue.Add(&item{key, obj, state})

it's storing the whole object
Then later it's using it to retrieve it by key

elastic-agent-autodiscover/kubernetes/watcher.go

Line 275 in e092f62

o, exists, err := w.store.GetByKey(key)

[swiatekm]

Having reviewed that code, I think it's fine and doesn't make any copies. obj is always a pointer to a k8s resource, so putting it in the queue doesn't copy it. The reason we need it is that we want to run handlers for delete events - and in that case, the object may not exist in the store anymore at the time our handler runs.

[henrikno]

From my understanding, the updates keep coming in, then you add them to the queue with a reference to that version of the object.
If there are multiple updates coming in quickly, by the time it's getting "processed" it might already have been updated. But, we only care about the latest version, not each change, so we look up the current version in the store. But since we keep a reference to the old version in the queue it can't be GCed. So the only case we care about older versions is if it's a delete, so we should only keep it if it's a delete.

[swiatekm]

That's a fair observation, but I doubt this makes a significant difference in practice. It's rare to get updates for the same object so quickly, and worst case we just hold on to some references for a bit longer. This change would make the code more complicated. I'm not convinced it's worth it, would need to see evidence that it actually makes a difference.