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

Avoid reloads implementing Equaler interface #862

Merged
merged 2 commits into from
Jun 16, 2017
Merged

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Jun 14, 2017

This change introduce two fixes:

  • reduces the reloads by half
  • the backend servers (endpoints) are not sorted

The last change address the issue kubernetes-retired/contrib#1827
that was explained in a kubecon session https://www.youtube.com/watch?v=iShh33WaoHw

Basically if an ingress controller is scaled to more than 1 the previous behavior was to use the same order of the upstream servers in all the instances. With this PR we randomize the order.

fixes kubernetes-retired/contrib#1827

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 14, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@aledbf aledbf changed the title WIP: Avoid reloads implementing Equals in structs WIP: Avoid reloads implementing Equaler interface Jun 14, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-3.5%) to 42.961% when pulling 655176b8c369a5b46dc3624cd9463f2c75eb9dce on aledbf:equals into 245e6b0 on kubernetes:master.

@aledbf aledbf changed the title WIP: Avoid reloads implementing Equaler interface Avoid reloads implementing Equaler interface Jun 15, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-2.1%) to 44.559% when pulling 92eeb78 on aledbf:equals into c6b5335 on kubernetes:master.

@aledbf aledbf merged commit e1bdce0 into kubernetes:master Jun 16, 2017
@aledbf aledbf deleted the equals branch June 19, 2017 20:05
@jcmoraisjr
Copy link
Contributor

Out of curiosity why reflect.DeepEqual wasn't an option here?

@aledbf
Copy link
Member Author

aledbf commented Jun 24, 2017

@jcmoraisjr no because with this change the slices are not sorted by IP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[nginx-ingress-controller] wrong balancing of requests for 20-30 seconds after configuration reload
5 participants