Skip to content
This repository has been archived by the owner on Oct 30, 2019. It is now read-only.

Connection agnostic crates #31

Closed
Thomasdezeeuw opened this issue Jul 27, 2018 · 6 comments
Closed

Connection agnostic crates #31

Thomasdezeeuw opened this issue Jul 27, 2018 · 6 comments

Comments

@Thomasdezeeuw
Copy link

Currently most crates use a concrete type for connections, often std::net::TcpStream, but to support async I/O often a different connection type is need, e.g. tokio::net::TcpStream.

For example take the postgres crate, which uses std::net::TcpStream for blocking I/O. But to support async I/O tokio-postgres was created which uses tokio::net::TcpStream.

I think it would be worthwhile to create a generic Connection trait, which crates like postgres (but also e.g. hyper) can use as an abstract connection type. This can be used for both blocking and async I/O, where std::io::ErrorKind::WouldBlock errors are handled by the user of the crate.

This has two advantages:

  1. Blocking and async I/O can be handled in the same code. This does require more attention then just writing blocking I/O code, i.e. the code needs to deal with an operation being restarted or it needs to support some kind of checkpointing, much like most Futures.
  2. Support for various async I/O types in the same crate. Tokio has it's own TcpStream, but so does Mio and so does the standard libary. Supporting a single generic connection type would support also three those types with the same code (assuming they implement the correct trait).

Possible design

A simple design would be the following.

use std::io::{Read, Write};

trait Connection: Read + Write { }

And add documentation that the Connection can return std::io::ErrorKind::WouldBlock which isn't really an error and the crate must gracefully handle it.

This assumes that calling read/write on a concrete type does wake itself when it returns WouldBlock, in the context of using futures. I believe this already the case for both Tokio and mio.

Future-io's AsyncRead and AsyncWrite

For async I/O the futures-io crate provides special read and write traits that hook into the task system. These trait could also be used rather then Read and Write. The downside of this is that blocking I/O won't be supported anymore.

Thoughts?

@sfackler
Copy link

For example take the postgres crate, which uses std::net::TcpStream for blocking I/O. But to support async I/O tokio-postgres was created which uses tokio::net::TcpStream.

The fact that these types are different is not what prevents the two crates from sharing code. Their implementations are fundamentally and significantly different.

@Thomasdezeeuw
Copy link
Author

@sfackler I'm not familiar with the codebase, however do you think that when using a generic connection type, like proposed, the code base could share more code?

For example I've writing a client libary with Redis to experiment with this idea. It uses a future like pattern where a Request object is returned when issueing a command (likely a query in case of postgres). This Request can be polled (without arguments) until completion (like a future), in case of blocking I/O this will be a single call. The Request itself is basically a state machine; first write the command and then read the response.

@seanmonstar
Copy link

hyper has done this for a while: it works with any transport type that is AsyncRead + AsyncWrite.

@sfackler
Copy link

when using a generic connection type, like proposed, the code base could share more code?

Nothing is preventing it from using a generic connection type right now! It wouldn't enable any more code reuse between the async and sync implementations, at least as currently designed.

@jsgf
Copy link

jsgf commented Jul 27, 2018

I think it would be worthwhile to create a generic Connection trait, which crates like postgres (but also e.g. hyper) can use as an abstract connection type. This can be used for both blocking and async I/O, where std::io::ErrorKind::WouldBlock errors are handled by the user of the crate.

I'm not quite sure what you mean here, but I think it effectively makes everything async (or at least as difficult to code as async, in terms of managing internal states with partial IO).

But postgres (or whatever) would need to handle it internally - the user of the postgres crate couldn't do it without knowing postgres's internal state. (If postgres gets a partial response from the socket and returns WouldBlock to its caller, what does it do with that partial response? What's the caller supposed to do with the WouldBlock?)

@Thomasdezeeuw
Copy link
Author

@jsgf When it reads a partial response it would store that in a bytes buffer, e.g. Vec<u8>, then it would return WouldBlock (or call read again to make the underlying connection returns WouldBlock). The caller needs to call poll again if WouldBlock is returned.

In case of blocking I/O most of them don't return partial responses, they return whatever is currently is in the buffer or blocking until something is written into it. That means that if only a part of a response is read the crate needs to call read again, which then blocks again.

For example: say when have Postgres client which sends an request. Then Postgres response with only half of the response initially. The client can then read the first half of the request and start parsing it. If it can't deal with partial requests it can just call read again and block until the entire request in read into a buffer.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants