update the proxy to use Tokio 0.3 #732
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's possible that there's a memory leak in the Tokio 0.2 version of `tokio::sync::watch`. This commit updates the service-profiles code to use Tokio 0.3's version of watch, whose internals are significantly different.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
discovery::buffer also needs this Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
turns out tower-rs/tower-http@926a64f made it more or less impossible for us to use the `tower-request-modifier` crate. it now returns services with `impl FnOnce` in their types, so we can't return it from `NewService` or `MakeService` impls. i'll fix this upstream eventually but for now let's just pin to the revision before it got that way.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
this fixes error types not downcasting since different versions of h2 are in tree Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
tests and metrics also rely on h2 error downcasting Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This branch updates the `linkerd2-drain` crate to use Tokio 0.3's synchronization primitives, and replace the `oneshot` channel with `watch` (removing the weird `future::Shared` nonsense). Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This tracks the change that added an `instrument` combinator in the main `tracing` crate. Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This branch updates `linkerd2-buffer`, and `linkerd2-proxy-discover`'s `buffer` module to use Tokio 0.3's MPSC channel rather than Tokio 0.2's. The rest of the proxy still uses Tokio 0.2, including the 0.2 runtime. Most of the Tokio synchronization primitives lost their `poll`-based interfaces in 0.3 as part of the move to intrusive lists of wakers for synchronization primitives (see tokio-rs/tokio#2325, tokio-rs/tokio#2509, and tokio-rs/tokio#2861). This change takes advantage of the inherently pinned nature of `async fn` and `async` blocks to avoid needing a separate heap allocation to store the waiter state for a task waiting on a synchronization primitive. However, it means that a synchronization primitive can _only_ be waited on when the future that waits on it is pinned --- otherwise, there is a potential dangling pointer. The `poll`-based APIs allowed waiting on synchronization primitives from unpinned contexts, so they were removed. To wait on the synchronization primitives from contexts that may not be pinned, such as `poll_ready`, it's necessary to add a `Pin<Box<...>>` around the future that's waiting on the synchronization primitive. This ensures that the future will not move while it's part of the wait list. It's important to note that this isn't an _additional_ allocation per waiter versus Tokio 0.2; instead, it's the same allocation that would have _always_ happened internally to the synchronization primitive in the 0.2 API. Now, it's moved outside of the `tokio::sync` type so that it can be avoided when used with `async`/`await` syntax, and added by the user when polling the sync primitives. Because we need to poll channel senders in `tower::Service` implementations' `poll_ready` functions, it was necessary to introduce our own bounded MPSC channel type that exposes a polling-based API. When the buffer's channel is full, we want to exert backpressure in `poll_ready`, so that callers such as load balancers could choose to call another service rather than waiting for buffer capacity. This branch adds a new `linkerd2-channel` crate that implements a pollable bounded channel, wrapping `tokio::sync`'s unbounded MPSC and using a `tokio::sync::Semaphore` to implement bounding. It's worth noting that this is, essentially, how `tokio::sync::mpsc`'s bounded channel is implemented --- it also uses the semaphore. However, our implementation exposes a `poll_ready` method by boxing the future that waits to acquire a semaphore permit, which the Tokio channel does not expose. This was factored out of PR #732. Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw
added a commit
that referenced
this pull request
Dec 4, 2020
This branch updates `linkerd2-buffer`, and `linkerd2-proxy-discover`'s `buffer` module to use Tokio 0.3's MPSC channel rather than Tokio 0.2's. The rest of the proxy still uses Tokio 0.2, including the 0.2 runtime. Most of the Tokio synchronization primitives lost their `poll`-based interfaces in 0.3 as part of the move to intrusive lists of wakers for synchronization primitives (see tokio-rs/tokio#2325, tokio-rs/tokio#2509, and tokio-rs/tokio#2861). This change takes advantage of the inherently pinned nature of `async fn` and `async` blocks to avoid needing a separate heap allocation to store the waiter state for a task waiting on a synchronization primitive. However, it means that a synchronization primitive can _only_ be waited on when the future that waits on it is pinned --- otherwise, there is a potential dangling pointer. The `poll`-based APIs allowed waiting on synchronization primitives from unpinned contexts, so they were removed. To wait on the synchronization primitives from contexts that may not be pinned, such as `poll_ready`, it's necessary to add a `Pin<Box<...>>` around the future that's waiting on the synchronization primitive. This ensures that the future will not move while it's part of the wait list. It's important to note that this isn't an _additional_ allocation per waiter versus Tokio 0.2; instead, it's the same allocation that would have _always_ happened internally to the synchronization primitive in the 0.2 API. Now, it's moved outside of the `tokio::sync` type so that it can be avoided when used with `async`/`await` syntax, and added by the user when polling the sync primitives. Because we need to poll channel senders in `tower::Service` implementations' `poll_ready` functions, it was necessary to introduce our own bounded MPSC channel type that exposes a polling-based API. When the buffer's channel is full, we want to exert backpressure in `poll_ready`, so that callers such as load balancers could choose to call another service rather than waiting for buffer capacity. This branch adds a new `linkerd2-channel` crate that implements a pollable bounded channel, wrapping `tokio::sync`'s unbounded MPSC and using a `tokio::sync::Semaphore` to implement bounding. It's worth noting that this is, essentially, how `tokio::sync::mpsc`'s bounded channel is implemented --- it also uses the semaphore. However, our implementation exposes a `poll_ready` method by boxing the future that waits to acquire a semaphore permit, which the Tokio channel does not expose. Finally, I've added some tests for the `linkerd2-channel` crate, based on Tokio's tests for the MPSC channel, modified where the APIs differ. This should help ensure we get similar behavior to what we expect from Tokio's MPSCs. This was factored out of PR #732. Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
olix0r
pushed a commit
to linkerd/linkerd2-proxy-api
that referenced
this pull request
Dec 4, 2020
This branch updates the `tonic` dependency to a Git dep on a branch that uses Tokio 0.3 and the latest versions of other dependencies, such as `bytes` and `tower`. This is necessary for linkerd/linkerd2-proxy#732. Hopefully a new `tonic` release will be out soon and we can change the git dependency back to a crates.io dependency... Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This branch updates the proxy to use Tokio 0.3 and the Tokio 0.3 versions of various ecosystem crates. This includes `tower` 0.4 and `bytes` 0.6, as well as the Tokio 0.3 versions of `tokio-util`, `hyper`, `tonic`, etc. Due to API changes in Tokio and in other dependencies, it was necessary to make some code changes as well as updating dependencies, but there should be no functional change. In particular: * Tokio's support for vectored IO changed significantly in 0.3, so this branch updates our use of `AsyncWrite` to participate in the new vectored write APIs * Hyper's HTTP/1.1 upgrade API changed in 0.14, so this branch changes the proxy's code for handling CONNECT to use the new API * Tokio removed support for some socket options, which now need to be set using `socket2` * Tokio removed the `poll_ready` method was removed from the bounded MPSC channel, so the proxy's buffers (`linkerd2-buffer` and the `buffer` module in `linkerd2-proxy-discover`) had to be switched to our own implementation (this merged separately, in PR #759). Several ecosystem crates have yet to be released, so we depend on them via Git dependencies for now. The patches in Cargo.toml can be removed as other dependencies publish their Tokio 0.3 versions.
hawkw
changed the title
olix0r
approved these changes
Dec 4, 2020
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Dec 15, 2020
This release features a change to the proxy's cache eviction strategy to ensure that clients (and their load balancers) are reused by new outbound connections. This can dramatically reduce memory consumption, especially for busy HTTP/1.1 clients. Also, the proxy's HTTP detection scheme has been made more robust. Previously, the proxy would perform a only single read to determine whether a TCP stream was HTTP, which could lead to false positives. Now, the proxy reads until at least the first newline, which is what the HTTP parser actually needs to make a proper determination. With this, the default dispatch timeouts have been increased to 5s to accomodate connection pools that may not issue an immediate request. Furthermore, this release includes an upgrade to Tokio v0.3 and its associated ecosystem. --- * update buffers to use Tokio 0.3 MPSC channels (linkerd/linkerd2-proxy#759) * Update the proxy to use Tokio 0.3 (linkerd/linkerd2-proxy#732) * Rename DetectHttp to NewServeHttp (linkerd/linkerd2-proxy#760) * http: more consistent names for body types (linkerd/linkerd2-proxy#761) * io: simplify the `Io` trait (linkerd/linkerd2-proxy#762) * trace: nicer traces in tests, clean up trace configuration (linkerd/linkerd2-proxy#766) * Ensure that services are held as long they are being used (linkerd/linkerd2-proxy#767) * outbound: add stack tests for http (linkerd/linkerd2-proxy#765) * cache: Ensure that actively held services are not evicted (linkerd/linkerd2-proxy#768) * cache: Only spawn a single task per cache entry (linkerd/linkerd2-proxy#770) * test: make integration tests shut up (linkerd/linkerd2-proxy#771) * metrics: Add support for microsecond counters (linkerd/linkerd2-proxy#772) * Add a protocol label to stack metrics (linkerd/linkerd2-proxy#773) * detect: Make protocol detection more robust (linkerd/linkerd2-proxy#744)
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Dec 15, 2020
This release features a change to the proxy's cache eviction strategy to ensure that clients (and their load balancers) are reused by new outbound connections. This can dramatically reduce memory consumption, especially for busy HTTP/1.1 clients. Also, the proxy's HTTP detection scheme has been made more robust. Previously, the proxy would perform a only single read to determine whether a TCP stream was HTTP, which could lead to false positives. Now, the proxy reads until at least the first newline, which is what the HTTP parser actually needs to make a proper determination. With this, the default dispatch timeouts have been increased to 5s to accomodate connection pools that may not issue an immediate request. Furthermore, this release includes an upgrade to Tokio v0.3 and its associated ecosystem. --- * update buffers to use Tokio 0.3 MPSC channels (linkerd/linkerd2-proxy#759) * Update the proxy to use Tokio 0.3 (linkerd/linkerd2-proxy#732) * Rename DetectHttp to NewServeHttp (linkerd/linkerd2-proxy#760) * http: more consistent names for body types (linkerd/linkerd2-proxy#761) * io: simplify the `Io` trait (linkerd/linkerd2-proxy#762) * trace: nicer traces in tests, clean up trace configuration (linkerd/linkerd2-proxy#766) * Ensure that services are held as long they are being used (linkerd/linkerd2-proxy#767) * outbound: add stack tests for http (linkerd/linkerd2-proxy#765) * cache: Ensure that actively held services are not evicted (linkerd/linkerd2-proxy#768) * cache: Only spawn a single task per cache entry (linkerd/linkerd2-proxy#770) * test: make integration tests shut up (linkerd/linkerd2-proxy#771) * metrics: Add support for microsecond counters (linkerd/linkerd2-proxy#772) * Add a protocol label to stack metrics (linkerd/linkerd2-proxy#773) * detect: Make protocol detection more robust (linkerd/linkerd2-proxy#744)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch updates the proxy to use Tokio 0.3 and the Tokio 0.3
versions of various ecosystem crates. This includes
tower0.4 andbytes0.6, as well as the Tokio 0.3 versions oftokio-util,hyper,tonic, etc. Due to API changes in Tokio and in other dependencies, itwas necessary to make some code changes as well as updating
dependencies, but there should be no functional change.
In particular:
branch updates our use of
AsyncWriteto participate in the newvectored write APIs
proxy's code for handling CONNECT to use the new API
set using
socket2poll_readymethod was removed from the boundedMPSC channel, so the proxy's buffers (
linkerd2-bufferand thebuffermodule inlinkerd2-proxy-discover) had to be switched toour own implementation (this merged separately, in PR update buffers to use Tokio 0.3 MPSC channels #759).
Several ecosystem crates have yet to be released, so we depend on them
via Git dependencies for now. The patches in Cargo.toml can be
removed as other dependencies publish their Tokio 0.3 versions.
Performance testing indicates that there may be a bit of an improvement
in tail latencies (especially above p99) on the new Tokio: