Failed to port Mononoke to `tokio-0.2`; experience report

I recently attempted to port Mononoke over from using tokio-0.1 Runtimes to using tokio-0.2 Runtimes, while cleaning up the test cases and fixing some of the tech debt in hgcli. I have had to bring this work to a close without landing the stack), because the available backward compatibility shims aren't good enough to let me just swap out the Runtimes, but also require me to rewrite all the code uses other parts of the tokio ecosystem.

tokio Crates in use

In Mononoke, as well as the runtime, we use tokio_codec for FramedRead/FramedWrite, tokio_io for AsyncRead/AsyncWrite and File, tokio_openssl for SSL connections, tokio_process for subprocesses, tokio_timer for timers, and tokio_net for TCP connections. This is not an exhaustive list of tokio family crates we use, because I've ignored things like tokio-uds and tokio-core which have been folded into other crates in newer tokio-0.1 releases

While porting the runtime dependent bits wasn't challenging, the rest caused me various degrees of difficulty:

Core problem

Underlying all this is that the various tokio-* crates expect to be able to access a tokio-0.1 runtime for calls like tokio::spawn. As a result, if you swap out the runtime to a tokio-0.2 runtime and add compat() calls in the right places, the code compiles and runs, but fails at runtime with errors about tokio being "shutdown".

Porting difficulty

Easy to handle

tokio_timer and tokio_openssl are easy to port to the tokio-0.2 versions - the API has changed, but for timer, it's easy to do the conversion, and for tokio_openssl, there are relatively few call sites that depend on the API directly.

Anything that uses futures::future, futures::stream or futures::sink ports in the obvious fashion.

Users of mpsc and oneshot are simple to port; our internal FutureExt trait is either obsoleted by the new APIs, or trivial to replace with new code (async/await and the new _concurrent combinators in StreamExt make this particularly easy).

Annoying, but manageable

tokio_net from tokio-0.1 depends on a tokio-0.1 executor. The tokio-0.2 version has an incompatible API; however, there are only a small number of users of tokio_net, so porting is manageable. If we'd made heavier direct use of tokio_net, the lack of a usable compatibility shim to the traits from tokio-0.1 that the old TcpStream implements would have caused fun.

tokio_codec is not significantly changed, but causes ripple-through with our internal async_compression crate - this can be fixed by using the public async_compression crate from crates.io.

tokio_process has migrated into tokio_net (for some reason not clear to me), and is now undocumented as a result. This can be worked around by ample reading of the code, and I assume this will be fixed soon as an oversight (if it's not fixed up in the next alpha release of tokio-0.2, I will raise an issue in the tokio issue tracker to ensure that it doesn't get forgotten).

Hard, help needed.

tokio_io and futures::io in the new world have different AsyncRead and AsyncWrite traits; there is no compatibility shim between old (0.1) tokio-io Async* traits and new (0.2) tokio-io Async* traits.

Further, tokio-0.1 AsyncRead etc traits inherit from their std::io sync counterparts - the tokio-0.2 versions do not.

On the compatibility side, the futures-0.3 Async* traits are not compatible with the tokio-0.2 Async* traits; as the compatibility shims are between tokio-0.1 Async* and futures-0.3 Async*, this requires a complete rewrite of all I/O code (including things that depend upon tokio-fs::File) to tokio-0.2, and that then ripples through into tokio_codec code.

And finally, the compat() AsyncRead implementation does not also implement the various std::io traits - BufRead, Read, etc - so code written against tokio-0.1 that requires some of the std::io traits as well as AsyncRead or AsyncWrite cannot be made to work with the compatibility shims.

Path forward

There are two chunks of work that would make resuming this port to tokio-0.2 much simpler:

  1. Write deeper compatibility shims between the tokio-0.1 and tokio-0.2 runtimes, so that I can just swap out Runtime and not the other parts of our tokio-0.1 code in order to end up on tokio-0.2. This would enable tokio-0.1's versions of tokio_timer, tokio_net etc to run on tokio-0.2's Runtime.
  2. Chase down the discrepancies between tokio::io and futures::io in tokio-0.2 and futures-0.3 so that I have a full set of compatibility shims available between tokio-0.1 AsyncRead, std::io::BufRead etc and modern AsyncRead, AsyncBufRead etc, and do not have to choose between tokio-0.2-friendly AsyncRead and futures-0.3 AsyncRead etc.

With those two in place, I could resume my efforts, and try to get Mononoke onto a modern tokio - for now, though, the amount of code to rewrite is too high to be anything other than a whole-team "stop work and rewrite" effort.

2 Likes

Thanks for the report. A smooth upgrade path has not been a focus yet as 0.2 is still under active dev. However, I guess now that we have a concrete experience report it would be a good time to start.

I opened an issue to track, could we continue discussion there? https://github.com/tokio-rs/tokio/issues/1549

Could you expand on which std traits you expected to be implemented?

Regarding the doc problems, I think we are hitting issues with docs.rs. This is the life of requiring nightly rust...

You can always build the docs locally. This would be easier than browsing code.

1 Like

I'll go into detail on specific problems in the issue tracker, leaving this thread for general discussion of my experience.

Thanks, BTW, for all the work you've done on tokio - having been exposed to quite a bit of 0.2 in this work, it's had a lot of quality of life improvements done since 0.1, and it's obvious that when I can succeed in this port, it'll make our lives better.