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

Support configurable reconcile rates #380

Merged
merged 3 commits into from
Dec 16, 2021
Merged

Conversation

negz
Copy link
Member

@negz negz commented Sep 25, 2021

Description of your changes

Partially addresses crossplane/crossplane#2595 - see that issue for details.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

I tested this together with crossplane/crossplane#2602. See crossplane/crossplane#2568 (comment) for details.

Signed-off-by: Nic Cope <[email protected]>
The default in controller-runtime is to use the 'configmapsleases' strategy,
which takes a lock using both a ConfigMap and a Lease. This is done for backward
compatibility. For reasons that are not immediately clear to me (presumably some
kind of saturation) we find that at --max-reconcile-rate=100 we start to see
leadership losses because we can't renew our lease before the 10 second renewal
deadline is up. This should be a safe change since we've been using
configmapleases for many releases, so I don't expect dropping configmaps to
affect anyone. (We also don't create more than one replica by default.)

Signed-off-by: Nic Cope <[email protected]>
Copy link
Collaborator

@Feggah Feggah left a comment

Choose a reason for hiding this comment

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

LGTM!

@negz negz merged commit df5cb19 into crossplane-contrib:master Dec 16, 2021
@Feggah Feggah mentioned this pull request Dec 22, 2021
2 tasks
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