update Hyper and tokio::{io, net} dependencies#510
Merged
hawkw merged 57 commits intomaster-tokio-0.2from May 13, 2020
Merged
Conversation
This does **not** update the payload module to `std::future`, since we are still on the futures 0.1 version of Hyper. It only updates the `tower-service` impls. This will be necessary to bring back the HTTP clients. Signed-off-by: Eliza Weisman <[email protected]>
This does **not** update the payload module to `std::future`, since we are still on the futures 0.1 version of Hyper. It only updates the `tower-service` impls. This will be necessary to bring back the HTTP clients. Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
proxy still not compile-y; uses old hyper in admin& elsewhere and also some stuff is just broken, but the io & transport crates mostly build on their own. Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
whoops, this broke "everything" Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
The `BoxedIo` trait used to have a special `shutdown_write` method that would call `std::net::TcpStream::shutdown(Shutdown::Write)`. This was added because Tokio 0.1's `<TcpStream as AsyncWrite>::shutdown` did _not_ call this method; it simply returned `Ok(())`. However, Tokio 0.2's `<TcpStream as AsyncWrite>::poll_shutdown` *does* call `std::net`'s `TcpStream::shutdown`. Since `BoxedIo` was ported directly from the 0.1 code and still called both `AsyncWrite::poll_shutdown` _and_ `Io::shutdown_write`, we would try to shut down the stream twice, and the second call would fail with a "transport endpoint is not connected" error. This broke the `tls_accept` tests, which expected there to be no error. This commit removes the `Io::shutdown_write` method, since Tokio's `TcpStream` now does what we want. Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Contributor
Author
|
I know this branch is kind of massive. However, this should unblock pretty much all the remaining update work. Now that we're on the |
olix0r
reviewed
May 12, 2020
olix0r
approved these changes
May 12, 2020
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
kleimkuhler
approved these changes
May 13, 2020
Contributor
kleimkuhler
left a comment
There was a problem hiding this comment.
Looks good! Left a few non-blocking comments
This file contains hidden or 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
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 Hyper dependency to 0.13 (and thus also updates
http/http-body), and updates all uses ofAsyncReadandAsyncWriteandtokio::nettypes to use the Tokio 0.2 versions.Thisalso implies updating
bytesfrom 0.4 to 0.5. Unfortunately, thischange is rather massive, but all these dependencies are pretty tightly
coupled. It wasn't really possible to upgrade them incrementally the way
it was for code in Linkerd, at least not without forking and patching the
dependencies, which I wanted to avoid.
Note on Test Support
This branch also makes a great deal of changes to the test support code
in
linkerd2-app-integration. This is necessary because this code nolonger compiled after updating Hyper and the IO traits. When practical,
I've tried to make this a direct translation of the existing code.
I think there's a lot of opportunity to improve this code and reduce
complexity. In particular, the test-support clients work by spawning a
background thread with a Tokio runtime on it, and sending work to that
task and responses back over channels. This is so we can present a
synchronous API to the tests themselves, and write straight-line code in
tests rather than nested futures. In an async/await world, that's kind
of unnecessary. We could switch the integration tests to be async
functions written using
#[tokio::test], and remove the complexity ofmanaging multiple background threads and runtimes for each test client
(and server). We would probably still want to isolate the test proxy
in its own runtime as we do currently, but we could run all the test
support code in the main test thread's runtime and not have to do all
this messing around with threads and channels.
However, I didn't do this kind of refactoring in this PR, beyond adding
async versions of some of the existing test support APIs (which still
use channels internally). These were added for use in tests which have
to call into async APIs from other crates and therefore need to be
async fns. I wanted to avoid totally rewriting the test support codein this branch, however, since it's already a huge change.
Note on IO Trait Changes
The
BoxedIotrait used to have a specialshutdown_writemethod thatwould call
std::net::TcpStream::shutdown(Shutdown::Write). This wasadded because Tokio 0.1's
<TcpStream as AsyncWrite>::shutdowndidnot call this method; it simply returned
Ok(()). However, Tokio0.2's
<TcpStream as AsyncWrite>::poll_shutdowndoes callstd::net'sTcpStream::shutdown. SinceBoxedIowas ported directlyfrom the 0.1 code and still called both
AsyncWrite::poll_shutdownand
Io::shutdown_write, we would try to shut down the stream twice,and the second call would fail with a "transport endpoint is not
connected" error. This broke the
tls_accepttests, which expectedthere to be no error.
This commit removes the
Io::shutdown_writemethod, since Tokio'sTcpStreamnow does what we want.Signed-off-by: Eliza Weisman [email protected]