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

Add PollIntervalJitter to Options #684

Closed
wants to merge 1 commit into from

Conversation

max-melentyev
Copy link

Jitter is useful when dealing with large number of resources to avoid load spikes.

I'm going to use this option in further provider-aws PRs.

@max-melentyev max-melentyev requested review from a team as code owners April 3, 2024 16:50
Jitter is useful when dealing with large number of resources to avoid load spikes.

I'm going to use this option in further provider-aws PRs.

Signed-off-by: Max Melentyev <[email protected]>
@negz
Copy link
Member

negz commented Apr 4, 2024

I'm going to use this option in further provider-aws PRs.

Could you open a PR with an example? More context around what you're trying to achieve would help review this PR.

Is the idea to use this with https://github.com/crossplane/crossplane-runtime/blob/v1.15.1/pkg/reconciler/managed/reconciler.go#L566?

I'm wondering whether we really need to be able to plumb down a configurable amount of jitter, or whether an appropriate amount could be derived from the poll interval.

@max-melentyev
Copy link
Author

Hi @negz , thanks for reviewing!

I've submitted one of the PRs I mentioned, it has some details: crossplane-contrib/provider-aws#2030. Along with custom poll interval for ec2 instances and route53 records we want to set jitter for them to avoid polling large batches at the same time.

whether an appropriate amount could be derived from the poll interval

With relatively low poll intervals (<= 30m) we just have hardcoded jitters equal pollInterval / 2. Though the plan is later to increase these poll intervals even further up to several hours. And in this case it will make sense to have jitters less than pollInterval / 2. I'm also not sure if jitter is needed for other resources that have short poll intervals.

@max-melentyev
Copy link
Author

Another thing to consider: I think provider-aws will still require custom controller.Options struct that extends crossplane-runtime Options to let controllers subscribe to EventBridge events to trigger reconciliations for their managed resources. So I can add jitter to that struct if it doesn't make sense for other providers.

@negz
Copy link
Member

negz commented Apr 4, 2024

Another thing to consider: I think provider-aws will still require custom controller.Options struct that extends crossplane-runtime Options to let controllers subscribe to EventBridge events to trigger reconciliations for their managed resources.

My preference would be to start there. If we then find it ends up being used a lot we could add it to this widely used struct.

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.

2 participants