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

ingress: Add power-of-two-random-choices enhancement #665

Conversation

Miciah
Copy link
Contributor

@Miciah Miciah commented Feb 23, 2021

This enhancement enables cluster administrators to configure ingress to use the "Power of Two Random Choices" balancing algorithm for more even balancing after router reloads.

### Goals

1. Enable cluster administrators to configure HAProxy to use Power of Two Random Choices.
2. Enable application developers configure HAProxy to use Power of Two Random Choices for their Routes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
2. Enable application developers configure HAProxy to use Power of Two Random Choices for their Routes.
2. Enable application developers to configure HAProxy to use Power of Two Random Choices for their Routes.


### User Stories

#### As a cluster administrator, I have an IngressController, it has Routes that update frequently, and I want to traffic to be balanced after a reload.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#### As a cluster administrator, I have an IngressController, it has Routes that update frequently, and I want to traffic to be balanced after a reload.
#### As a cluster administrator, I have an IngressController, it has Routes that update frequently, and I want traffic to be balanced after a reload


Using the Power of Two Random Choices may improve balancing in this scenario.

#### As a cluster administrator, I have a large number of IngressController Pod replicas and a Route with occasional traffic spikes, and I want to ensure that traffic spikes are balanced across the Route's backend servers.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#### As a cluster administrator, I have a large number of IngressController Pod replicas and a Route with occasional traffic spikes, and I want to ensure that traffic spikes are balanced across the Route's backend servers.
#### As a cluster administrator, I have a large number of IngressController Pod replicas and a Route with occasional traffic spikes, and I want to ensure that traffic spikes are balanced across the Route's backend servers

If the feature gate is enabled, the operator configures the Deployment by
specifying "random" for `ROUTER_LOAD_BALANCE_ALGORITHM`, changing the default
algorithm to Power of Two Random Choices.

Copy link
Contributor

Choose a reason for hiding this comment

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

The router also needs to be changed to allow a random value for the haproxy.router.openshift.io/balance route annotation, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup! Thanks!

@Miciah Miciah force-pushed the ingress-add-power-of-two-random-choices-enhancement branch from 3361e30 to 4125062 Compare February 23, 2021 20:32
@knobunc
Copy link
Contributor

knobunc commented Mar 22, 2021

/approve

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: knobunc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 22, 2021
@sgreene570
Copy link
Contributor

sgreene570 commented Mar 22, 2021

/lgtm
/hold for others to review
(But feel free to drop the hold Miciah)

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 22, 2021
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 22, 2021
sgreene570 added a commit to sgreene570/api that referenced this pull request Mar 23, 2021
Add a new feature gate, `PowerOfTwoRandomChoicesBalancing`, that is
disabled by default. Enabling this feature gate will tell the
OpenShift Router (HAProxy) to use the "Power of Two Random Choices"
balancing algorithm by default, in place of the current `leastconn`
default balancing algorithm.

See https://www.haproxy.com/blog/power-of-two-load-balancing/
as well as openshift/enhancements#665
for more context.
@Miciah Miciah force-pushed the ingress-add-power-of-two-random-choices-enhancement branch from 4125062 to 20637b9 Compare March 29, 2021 19:18
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 29, 2021
@sgreene570
Copy link
Contributor

/retest

@sgreene570
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 29, 2021
@Miciah
Copy link
Contributor Author

Miciah commented Mar 29, 2021

Rebased.

Never mind; wrong PR.

@frobware
Copy link
Contributor

frobware commented Apr 2, 2021

/lgtm
/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 2, 2021
@openshift-merge-robot openshift-merge-robot merged commit 66dce03 into openshift:master Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants