kubernetes / sample-controller

Repository for sample controller. Complements sample-apiserver
Apache License 2.0
3.14k stars 1.08k forks source link

[question] AddRateLimited or Add directly #63

Closed gaocegege closed 4 years ago

gaocegege commented 4 years ago

Hi I have a question about the usage of workqueue in the sample controller.

We are using workqueue.RateLimitingInterface as the work queue, but we only use AddRateLimited in processNextWorkItem.

        // Run the syncHandler, passing it the namespace/name string of the
        // Foo resource to be synced.
        if err := c.syncHandler(key); err != nil {
            // Put the item back on the workqueue to handle any transient errors.
+           c.workqueue.AddRateLimited(key)
            return fmt.Errorf("error syncing '%s': %s, requeuing", key, err.Error())
        }

In enqueueFoo, we use Add instead. Is there any convention about it?

// enqueueFoo takes a Foo resource and converts it into a namespace/name
// string which is then put onto the work queue. This method should *not* be
// passed resources of any type other than Foo.
func (c *Controller) enqueueFoo(obj interface{}) {
    var key string
    var err error
    if key, err = cache.MetaNamespaceKeyFunc(obj); err != nil {
        utilruntime.HandleError(err)
        return
    }
+   c.workqueue.Add(key)
}
YesterdayxD commented 4 years ago

I think the reason is that only it exists error , call c.workqueue.AddRateLimited(key) .

// Put the item back on the workqueue to handle any transient errors.

But if it exists any informer update, call enqueueFoo.Don't need error occurs.

fejta-bot commented 4 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot commented 4 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten

gaocegege commented 4 years ago

/close

k8s-ci-robot commented 4 years ago

@gaocegege: Closing this issue.

In response to [this](https://github.com/kubernetes/sample-controller/issues/63#issuecomment-605761125): >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.