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

balance: Implement a Peak-EWMA load metric #76

Merged
merged 50 commits into from
Jun 7, 2018

Conversation

olix0r
Copy link
Collaborator

@olix0r olix0r commented May 31, 2018

The balancer provides an implementation of two load balancing strategies: RoundRobin and
P2C+LeastLoaded. The round-robin strategy is extremely simplistic and not sufficient for
most production systems. P2C+LL is a substantial improvement, but relies exclusively on
instantaneous information.

This change introduces P2C+PeakEWMA strategy. P2C+PE improves over P2C+LL by maintaining
an exponentially-weighted moving average of response latencies for each endpoint so that
the recent history directly factors into load balancing decisions. This technique was
pioneered by Finagle for use at Twitter. Finagle's P2C+PE implementation was
referenced heavily while developing this.

The provided demo can be used to illustrate the differences between load balacing
strategies. For example:

REQUESTS=50000
CONCURRENCY=50
ENDPOINT_CAPACITY=50
MAX_ENDPOINT_LATENCIES=[1ms, 10ms, 10ms, 10ms, 10ms, 100ms, 100ms, 100ms, 100ms, 1000ms, ]
P2C+PeakEWMA
  wall   15s
  p50     5ms
  p90    56ms
  p95    78ms
  p99    96ms
  p999  105ms
P2C+LeastLoaded
  wall   18s
  p50     5ms
  p90    57ms
  p95    80ms
  p99    98ms
  p999  857ms
RoundRobin
  wall   72s
  p50     9ms
  p90    98ms
  p95   496ms
  p99   906ms
  p999  988ms

@olix0r olix0r self-assigned this May 31, 2018
Update the demo to support response tracking

Also, randomize response latencies to be a more interesting simulation.

use Load;

pub struct PeakEWMA<S, T> {
Copy link
Member

Choose a reason for hiding this comment

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

NIT: I believe that correct Rust style would call this PeakEwma.

@hawkw
Copy link
Member

hawkw commented May 31, 2018

🙌

@olix0r
Copy link
Collaborator Author

olix0r commented May 31, 2018

I've improved the demo somewhat (moved to new tokio, randomized latencies, constrained concurrency, used the Measure api in both p2c cases), which helps to illustrate the performance benefits of peak ewma (p2c+pe):

p2c+pe samples: 1000000
p2c+pe p50:  302
p2c+pe p90:  463
p2c+pe p95:  483
p2c+pe p99:  499
p2c+pe p999: 1811
p2c+ll samples: 1000000
p2c+ll p50:  305
p2c+ll p90:  470
p2c+ll p95:  491
p2c+ll p99:  1457
p2c+ll p999: 1946
rr samples: 1000000
rr p50:  319
rr p90:  495
rr p95:  1142
rr p99:  1829
rr p999: 1983

In order to better-utilize the reactor, and to ensure that the load
balancer does not account for time waiting for the client to read
responses, we wrap the balancer in a buffer.
@olix0r
Copy link
Collaborator Author

olix0r commented May 31, 2018

After improving the test, we see much more pronounced benefit:

REQUESTS=1000000 CONCURRENCY=1000
p2c+pe p50:  30ms
p2c+pe p90:  46ms
p2c+pe p95:  48ms
p2c+pe p99:  51ms
p2c+pe p999: 74ms
p2c+ll p50:  30ms
p2c+ll p90:  46ms
p2c+ll p95:  48ms
p2c+ll p99:  50ms
p2c+ll p999: 1436ms
rr p50:  32ms
rr p90:  50ms
rr p95:  1004ms
rr p99:  1802ms
rr p999: 1980ms

Previously, the load balancer accounted for time wiating for the SendRequests tasks to process each future. Now this is done in separate tasks (in a threadpool).

let clock = clock::Clock::new_with_now(Now(time.clone()));

let mut enter = enter().expect("enter");
clock::with_default(&clock, &mut enter, |_| {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@carllerche this works fine! though, ergonomically, this feels like boilerplate. I think we could probably package up a default mock timer that handles this.

Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

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

Looking good. Minor requests inline.

@@ -8,6 +8,7 @@ extern crate indexmap;
#[cfg(test)]
extern crate quickcheck;
extern crate rand;
extern crate tokio;
Copy link
Member

Choose a reason for hiding this comment

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

This should not depend on Tokio proper. It looks like it needs tokio-timer for clock.

// ===== impl PeakEwma =====

impl<D: Discover> WithPeakEwma<D, NoInstrument> {
pub fn new(discover: D, decay: Duration) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

Is it going to be common to create an instance with NoInstrument?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably not enough to warrant the split

@olix0r olix0r changed the base branch from ver/balance-track-rsp-h2 to master June 6, 2018 19:18
@olix0r olix0r changed the title wip: Balance: Implement a Peak-EWMA load metric balance: Implement a Peak-EWMA load metric Jun 6, 2018
Cargo.toml Outdated
@@ -21,3 +21,8 @@ log = "0.4.1"
env_logger = { version = "0.5.3", default-features = false }
tokio-timer = "0.1"
futures-cpupool = "0.1"

[patch.crates-io]
tokio = { git = "https://github.com/tokio-rs/tokio", branch = "timer-abstract-now" }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should probably block merging this until this is released

Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to get it merged, but now CI is hanging!

@carllerche
Copy link
Member

Approved, but yeah. We are waiting for the upcoming tokio release (whenever CI decides to cooperate).

@olix0r olix0r merged commit fcdc9d2 into master Jun 7, 2018
@olix0r olix0r deleted the ver/balance-track-rsp-h2-peak-ewma branch June 7, 2018 06:16
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.

None yet

3 participants