Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce RCR Throttling #1545

Merged
merged 4 commits into from
Feb 8, 2021
Merged

Conversation

realshuting
Copy link
Member

Related issue

Fixes #1497.

This PR adds a buffer to slow down report change request creation.

Signed-off-by: Shuting Zhao <shutting06@gmail.com>
@@ -86,6 +82,7 @@ func NewReportChangeRequestGenerator(client *policyreportclient.Clientset,
polListerSynced: polInformer.Informer().HasSynced,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), workQueueName),
dataStore: newDataStore(),
requestCreator: newChangeRequestCreator(dclient, 3*time.Second, log.WithName("requestCreator")),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I set the default ticker interval to 3 seconds (the changeReqeustCreator will aggregate buffered requests every 3s). Should we allow this to be tuned dynamically across various loads of clusters?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if there are no buffered requests will there be an overhead?

Why not use a fixed size pool of workers that block on the queue instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there are no buffered requests will there be an overhead
No.

We can use a fixed size pool, but also need to set a "timeout" interval to flush the queue. So I buffer the requests and aggrate them every fixed interval.

Copy link
Member Author

@realshuting realshuting left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a question, see comments.

Signed-off-by: Shuting Zhao <shutting06@gmail.com>
Signed-off-by: Shuting Zhao <shutting06@gmail.com>
Signed-off-by: Shuting Zhao <shutting06@gmail.com>
Copy link
Member

@JimBugwadia JimBugwadia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! We can discuss the worker pattern and sizing separately for scale and availability as part of the HA design.

@@ -86,6 +82,7 @@ func NewReportChangeRequestGenerator(client *policyreportclient.Clientset,
polListerSynced: polInformer.Informer().HasSynced,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), workQueueName),
dataStore: newDataStore(),
requestCreator: newChangeRequestCreator(dclient, 3*time.Second, log.WithName("requestCreator")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if there are no buffered requests will there be an overhead?

Why not use a fixed size pool of workers that block on the queue instead?

@realshuting realshuting merged commit bd44dbf into kyverno:main Feb 8, 2021
@realshuting realshuting deleted the reduce_rcr_throttling branch February 8, 2021 19:23
vyankyGH pushed a commit that referenced this pull request Feb 9, 2021
* buffer report change requests

Signed-off-by: Shuting Zhao <shutting06@gmail.com>

* fix clusterReportChangeRequest

Signed-off-by: Shuting Zhao <shutting06@gmail.com>

* further reduce RCRs in background scan

Signed-off-by: Shuting Zhao <shutting06@gmail.com>
Signed-off-by: vyankatesh_neualto <vyankatesh@neualto.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Reduce to generate a single RCR per admission request
2 participants