outbound: Cache balancers within profile stack by olix0r · Pull Request #641 · linkerd/linkerd2-proxy

olix0r · 2020-08-29T04:15:09Z

In an upcoming change, we'd like to do service profile discovery within
the TCP-accept stack (and not the HTTP routing stack). But, today,
service profile discovery is tightly coupled to the HTTP middleware
implementations. This change splits the service profile layers (profile
discovery, http request profiles, and traffic splitting) into several
layers, so that the discovery logic is decoupled from the http-specific
request-routing middleware.

This change removes the balancer cache and the balancer-specific buffer
so that balancers are owned by the split layer. The buffer has been
moved outside of the split layer to drive all balancers in a split and
to make the split cloneable (for the retry middleware). All of this is
cached under the profile cache.

Breaking changes:

This likely breaks support for "external" service profiles, where the
proxy resolves service profilse for domains that it cannot resolve via
the resolver. This feature is not enabled by default and is considered
experimental. We'll have to take attention to at least document this in
upcoming releases.

Side-effects:

Latency improvements at concurrency
Compile time improved by 20-25% (integration tests in CI ~20m => ~15m);

This change updates and extends the diagnostic stack checks that we use during development. No functional changes.

The `HasDestination` trait isn't particularly useful, as it's basically just `AsRef<Addr>`. This change updates the `GetRoutes` signature to support this; and it updates the inbound target type to store an `Addr` instead of an `Option<NameAddr>` (so `Target` will be suitable for this in an upcoming change).

We need tower-rs/tower@ad348d8 for an upcoming change. This PR updates the tower dependency in anticipation of this change. The `Balance` constructor has changed.

hawkw

this looks great to me so far, nice job on the latency reduction!

linkerd/service-profiles/src/split.rs

linkerd/app/inbound/src/lib.rs

linkerd/app/outbound/src/lib.rs

linkerd/service-profiles/src/http/route_request.rs

hawkw · 2020-08-31T17:32:36Z

linkerd/service-profiles/src/split.rs

+    let rng = SmallRng::from_entropy();
+    layer::mk(move |inner| NewSplit {
+        inner,
+        rng: rng.clone(),


should we be doing something like the second example here? i'll admit i'm not too familiar w/ rand's APIs...

@hawkw is that the right link? if so, i'm not sure what you're suggesting...

whoops, it is not --- i meant to link to here: https://docs.rs/rand/0.7.3/rand/rngs/struct.SmallRng.html#examples

hawkw · 2020-08-31T17:33:11Z

linkerd/service-profiles/src/split.rs

+        let mut update = None;
+        while let Poll::Ready(Some(up)) = self.rx.poll_recv_ref(cx) {
+            update = Some(up.clone());
+        }


as above, not sure if we need to do this...

If the ready cache isn't ready, then we want to be notified when the profile changes, so we have to drive it to pending. In general, we should always be driving to not ready to be sure we're at the latest state.

In an upcoming change, we'd like to do service profile discovery within the TCP-accept stack (and not the HTTP routing stack). But, today, service profile discovery is tightly coupled to the HTTP middleware implementations. This change splits the service profile layers (profile discovery, http request profiles, and traffic splitting) into several layers, so that the discovery logic is decoupled from the http-specific request-routing middleware. This change removes the balancer cache and the balancer-specific buffer so that balancers are owned by the split layer. The buffer has been moved outside of the split layer to drive all balancers in a split and to make the split cloneable (for the retry middleware). All of this is cached under the profile cache. Breaking changes: This likely breaks support for "external" service profiles, where the proxy resolves service profilse for domains that it cannot resolve via the resolver. This feature is not enabled by default and is considered experimental. We'll have to take attention to at least document this in upcoming releases. Side-effects: * Latency improvements at concurrency ![image](https://user-images.githubusercontent.com/240738/91736211-2fd48280-eb62-11ea-8420-30e0242980a5.png) * Compile time improved by 20-25% (integration tests in CI ~20m => ~15m);

hawkw

looks lovely, ship it!

This release includes several major changes to the proxy's behavior: - Service profile lookups are now necessary and fundamental to outbound discovery for HTTP traffic. That is, if a service profile lookup is rejected, endpoint discovery will not be performed; and endpoint discovery must succeed for all destinations that are permitted by service profiles. This simplifies caching and buffering to reduce latency (especially under concurrency). - Service discovery is now performed for all TCP traffic, and connections are balanced over endpoints according to connection latency. - This enables mTLS for **all** meshed connections; not just HTTP. - Outbound TCP metrics are now hydrated with endpoint-specific labels. --- * outbound: Cache balancers within profile stack (linkerd/linkerd2-proxy#641) * outbound: Remove unused error type (linkerd/linkerd2-proxy#648) * Eliminate the ConnectAddr trait (linkerd/linkerd2-proxy#649) * profiles: Do not rely on tuples as stack targets (linkerd/linkerd2-proxy#650) * proxy-http: Remove unneeded boilerplate (linkerd/linkerd2-proxy#651) * outbound: Clarify Http target types (linkerd/linkerd2-proxy#653) * outbound: TCP discovery and load balancing (linkerd/linkerd2-proxy#652) * metrics: Add endpoint labels to outbound TCP metrics (linkerd/linkerd2-proxy#654)

olix0r added 30 commits August 26, 2020 22:35

fixup Cargo.toml

198e90b

wip

678e90d

wip

68da5ae

dynamic traffic split middleware

e0556b6

split layer

c36faac

wip

2412c65

dynamic request routes

aec1af3

Handle empty overrides

e4384b8

touchup: derive clone

ed91beb

touchup

6962e58

rename http::service to discover

673d1e8

move types around

83c4bf3

Rename profiles modules

a157e03

wip

2ef104d

trace => debug

2036545

Use ReadyCache in traffic split

24d88a1

outbound compiles

b952440

compiles without route metrics

69fa690

compiles; fallback tests fail

194be9e

debug

bd4a5b6

Update tower::ReadyCache to propagate error types

0ef01ea

fixup port

2774a01

restore test

e73531c

bump dep

a411188

Merge branch 'main' into ver/profile-split

46ffa48

-unused Into<Addr> for Target

ea39d34

undo rename

eca3920

svc: Update stack diagnostic checks

54b43b3

This change updates and extends the diagnostic stack checks that we use during development. No functional changes.

dst=>logical

8fb3014

olix0r added 14 commits August 31, 2020 14:53

update tower to primary repo

8b5d9da

Update tower to tower-rs/tower@ad348d8

58517b3

We need tower-rs/tower@ad348d8 for an upcoming change. This PR updates the tower dependency in anticipation of this change. The `Balance` constructor has changed.

Merge branch 'ver/tower' into ver/profile-split

e08bf22

fixup

ebc12cb

remove unused modules

c3e11c7

Remove needless http_ variable name prefixes

0012b53

fixup

cd68def

split: there must always be at least one target

7c017e3

split: Ensure there is at least one target in all cases

679600f

Merge branch 'main' into ver/profile-split

a10dc4e

comment

0e23979

outbound: fewer stack checks

37502fe

balance => concrete

917ca75

commentary

1f3cd56

hawkw reviewed Aug 31, 2020

View reviewed changes

olix0r added 4 commits August 31, 2020 17:35

commentary/clarity

ac6056a

fixup assertions

ea783c8

Commentary

3fbebc3

olix0r changed the title ~~Ver/profile split~~ outbound: Cache balancers within profile stack Aug 31, 2020

olix0r marked this pull request as ready for review August 31, 2020 18:15

olix0r requested a review from a team August 31, 2020 18:15

fixup comment

15b5de6

olix0r requested a review from hawkw August 31, 2020 18:27

hawkw approved these changes Aug 31, 2020

View reviewed changes

olix0r merged commit b89f58c into main Aug 31, 2020

olix0r deleted the ver/profile-split branch August 31, 2020 19:24

olix0r mentioned this pull request Sep 10, 2020

proxy: v2.109.0 linkerd/linkerd2#4957

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

outbound: Cache balancers within profile stack#641

outbound: Cache balancers within profile stack#641
olix0r merged 66 commits intomainfrom
ver/profile-split

olix0r commented Aug 29, 2020 •

edited

Loading

Uh oh!

hawkw left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hawkw Aug 31, 2020

Uh oh!

olix0r Aug 31, 2020

Uh oh!

hawkw Aug 31, 2020

Uh oh!

hawkw Aug 31, 2020

Uh oh!

olix0r Aug 31, 2020 •

edited

Loading

Uh oh!

hawkw left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

olix0r commented Aug 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hawkw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hawkw Aug 31, 2020

Choose a reason for hiding this comment

Uh oh!

olix0r Aug 31, 2020

Choose a reason for hiding this comment

Uh oh!

hawkw Aug 31, 2020

Choose a reason for hiding this comment

Uh oh!

hawkw Aug 31, 2020

Choose a reason for hiding this comment

Uh oh!

olix0r Aug 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hawkw left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

olix0r commented Aug 29, 2020 •

edited

Loading

olix0r Aug 31, 2020 •

edited

Loading