controller should drop duplicate re-queue requests

[ctron]

It looks to me as if the controller triggers exactly one event for each change that was detected on e.g. an "owned resource".

Assume you have 10 owned resources for each resource, and 10 managed resources. That makes 100 events and thus 100 reconciliations on the startup of an operator.

The same for scheduled/delayed reconcile outcomes.

IMHO the controller queue should queue up a reconcile request if there is no event yet scheduled for this resource, or if the scheduled event is further in the future than the new request.

[nightkr]

Assume you have 10 owned resources for each resource, and 10 managed resources. That makes 100 events and thus 100 reconciliations on the startup of an operator.

No, the scheduler ensures that each object can only have one spot in the queue.

This considers the controller's root objects, not the triggering objects (so 5 Pods owned by 1 Controller would still be debounced based on the Controller).

Once slight caveat is that objects can be rescheduled as soon as the reconciler has started (you don't need to wait for it to finish). This means that depending on timing, you might end up with ~3 reconciles per object on startup (one gets started immediately since the queue is empty, one gets queued based on a secondary trigger, one gets queued because you updated the object's status).

If you /are/ able to reproduce the OP's behaviour reliably then that's probably a bug, and I'd appreciate seeing it.

[ctron]

This is what I can see in the log:

[2020-09-07T13:00:29Z INFO  hawkbit_operator::controller] Reconcile: hawkbit-operator/default
[2020-09-07T13:00:29Z INFO  hawkbit_operator::controller] Reconcile: hawkbit-operator/default
[2020-09-07T13:00:29Z INFO  hawkbit_operator::controller] Reconcile: hawkbit-operator/default
[2020-09-07T13:00:29Z INFO  hawkbit_operator::controller] Reconcile: hawkbit-operator/default
[2020-09-07T13:00:30Z INFO  hawkbit_operator::controller] Reconcile: hawkbit-operator/default
[2020-09-07T13:00:30Z INFO  hawkbit_operator::controller] Reconcile: hawkbit-operator/default
[2020-09-07T13:00:30Z INFO  hawkbit_operator::controller] Reconcile: hawkbit-operator/default
[2020-09-07T13:00:30Z INFO  hawkbit_operator::controller] Reconcile: hawkbit-operator/default
[2020-09-07T13:00:30Z INFO  hawkbit_operator::controller] Reconcile: hawkbit-operator/default
[2020-09-07T13:00:30Z INFO  hawkbit_operator::controller] Reconcile: hawkbit-operator/default
…

It goes on for a while like this.

Taking a look at the code you referenced, it looks to me as if that has an issue in the case you return with a requeue-delay, as you currently have to.

Assume you return in the first reconcile loop with requeue=10s … then he next event would still be queued, before it is earlier. Resulting in the same 10s, that loop could go on for a while.

[nightkr]

In that case, the old queue entry should be reused. Is hawkbit_operator public?

[ctron]

In that case, the old queue entry should be reused. Is hawkbit_operator public?

Yes: https://github.com/ctron/hawkbit-operator

[ctron]

Maybe it would make sense do add some rate limiting feature, as in the Go version of the controller runtime:

• https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/ratelimiter/ratelimiter.go
• https://github.com/kubernetes/client-go/blob/master/util/workqueue/rate_limiting_queue.go

[clux]

Revisiting this old issue to ensure we are doing everything we can. Here is a brief overview of what we do:

• [x] deduplication in scheduler. yes, provided it happened before it ran
• [x] debouncing of events by a few seconds in the scheduler. could be better - moved to #1247
• [x] allow opt-in predicates to hard-filter per-input stream

For anything else there are still the standard approaches;

• use annotations on secondary streams to set checkpoints for the reconciler
• use status fields on the primary stream to create checkpoints for the reconciler
• gate check in the reconciler work that is guaranteed to be done to avoid duplicate heavy work

but these approaches are scarier since you run the risk of creating less idempotent reconcilers that can get into bad states (reconciler thinks it is done, and you cannot re-trigger it because it gate checks itself out of correcting the problem).

Debouncing

I feel the key question left for this issue is: Is our debouncing as good as it can be? Judging by the code comments, probably not, but this stuff is also hard. Ideally we want classical debouncing behavior:

• event comes in? schedule a reconciliation for the relevant object at now() + N seconds
• another event comes in before reconciliation is due? reschedule, for now() + N seconds
• N second elapses after last event? reconcile

5 seconds is probably OK given the heavy work that usually works, but if we ever manage to expose options for the Controller, then this would be a good thing to expose.

Leaving this open and resolving older comments that are now out of date. If someone wants to look at refining this, or experiment, you are welcome to, otherwise going to leave this as is.

[clux]

Closing this. Better scoped issue about how to improve scheduler debouncing linked in #1247