update the core proxy components to std::future and tower 0.3#482
Merged
hawkw merged 28 commits intomaster-tokio-0.2from Apr 22, 2020
Merged
update the core proxy components to std::future and tower 0.3#482hawkw merged 28 commits intomaster-tokio-0.2from
hawkw merged 28 commits intomaster-tokio-0.2from
Conversation
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]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
hawkw
changed the title
This was referenced Apr 21, 2020
Member
|
@hawkw what parts of this warrant the most attention in your opinion? |
Contributor
Author
Honestly? None of it warrants that much attention. Large chunks of it is just commenting stuff out, and the rest is fairly mechanical translation from the old |
olix0r
approved these changes
Apr 22, 2020
kleimkuhler
approved these changes
Apr 22, 2020
Contributor
kleimkuhler
left a comment
kleimkuhler
left a comment
There was a problem hiding this comment.
Looks good! Not sure how much cleanup you'd like to stay on top of now vs once everything is done, but most of my comments are small non-blockers.
Thanks @kleimkuhler! 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]>
hawkw
added a commit
that referenced
this pull request
Jun 12, 2020
Although the `Drain` type which powers the proxy's graceful shutdown machinery was updated in #482, we were not actually triggering graceful shutdowns. This is because the Hyper `ConnectionFuture` was wrapped in a compatibility adapter which did not allow access to the inner future so that we could call the `graceful_shutdown` method on it. When we updated to the `std::future` version of Hyper, the graceful shutdown code was not re-enabled. This branch puts back the call to `ConnectionFuture::graceful_shutdown`. It also re-enables the shutdown integration tests, which pass now. Signed-off-by: Eliza Weisman <[email protected]>
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
3 participants
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 core proxy components to use
std::futureandtower0.3. This includes most of theappcrate and its components,the
transportcrate, and some miscellaneous stuff inproxythat wasnecessary for everything else to work.
I know this is a huge PR, but unfortunately, the coupling between these
crates made it kind of necessary. In theory, the proxy's constituent
crates are fairly loosely coupled, and in most cases, we don't need to
make this kind of cross-cutting change, but using Tower traits as an
interface is a big part of how we are able to implement this loose
coupling. Given the constraint that the
master-tokio-0.2branch shouldalways compile, I don't think it's possible to do the
std::futureupdate without a branch like this.
Fortunately, although large, the change here is mostly pretty
mechanical. There's a lot of udpdating
Serviceimplementations to thenew Tower 0.3 version of the trait, including some slight tweaks to
future implementations to work with stack pinning. Some code (such as
tap and the controller client) had to be commented out to avoid having
to update additional proxy subsystems end-to-end in this branch; we will
update those components in follow-up PRs.
I haven't updated from tokio 0.1 async io to tokio 0.2 async IO; that
will be a later pass. This is because having both
hyper0.12 andtokio0.2 with the "net" feature in the dependency graph at the sametime pulls in incompatible
iovecversions and is very hard tountangle.
hyperand the io types (theAsyncRead/Writetraits,sockets, etc) will have to move together.
Once this PR lands and the new Tower version is wired all the way
through the core of the proxy, it should be pretty easy to update the
rest of the crates in separate, independent PRs. We can essentially go
through the stack layer-by-layer, and update each component. Once that's
done, then we can circle back and update Hyper and the IO traits to the
Tokio 0.2 versions.
Closes #474 (it's contained in this branch and can't be merged
independently without breaking stuff)