bearcove / loona

HTTP 1+2 in Rust, with io_uring & ktls
https://docs.rs/loona
Apache License 2.0
361 stars 13 forks source link

WriteOwned and write #72

Closed FrankReh closed 1 year ago

FrankReh commented 1 year ago

Hi. What's your take on whether or not writev should be interpreted as a write all?

I suspect most people would say it shouldn't be, as there is a count being returned.

So if an implementation wants to simulate a writev with a looping write, should it check the length of each write result and exit the loop early if the write wasn't complete?

And in the case of the owned API, should it have to rebuild the Vec so all the entries are there for the caller to reuse, perhaps after modifying the vec slice and one of the vec entry slices?

Since the input vec argument ownership is passed in, it seems reasonable to replace the entries per write but I haven't tried to play with that yet to see how easy the ownership is kept straight, without resorting to unsafe.

fasterthanlime commented 1 year ago

What's your take on whether or not writev should be interpreted as a write all?

You're asking in the context of designing tokio-uring interfaces?

It seems the consensus (pre-uring) is to let writev do partial writes and "return" immediately, letting the caller "advance buffers" themselves. My first thought is that it'd be strange if writev did mutate the vec of buffers, since I don't think other writev implementations do that - giving ownership is "just" to make sure that the buffer doesn't vanish from under io_uring.

Perhaps that discussion is better suited to some issue on tokio-uring? You can tag me there if you want.

FrankReh commented 1 year ago

Sorry, I was too obtuse. I noticed your implementation of writev didn't allow for the kernel to do partial writes, each iteration was calling write, not write_all, and on error, it wasn't necessarily returning the full vec as it was building a new vec along the way while being successful.

I felt bad to bring it up because I know you haven't asked for help or bug reports and distractions like this are expensive and perhaps not worth it.

fasterthanlime commented 1 year ago

I noticed your implementation of writev didn't allow for the kernel to do partial writes, each iteration was calling write, not write_all

Oh, nice catch!

and on error, it wasn't necessarily returning the full vec as it was building a new vec along the way while being successful.

That one I don't see - in the Err arm, it does out_list.extend(list), so it should have all the input buffers?

FrankReh commented 1 year ago

Gosh. I didn't know about extend and someone I didn't even see that line.

fasterthanlime commented 1 year ago

I felt bad to bring it up because I know you haven't asked for help or bug reports and distractions like this are expensive and perhaps not worth it.

Don't feel bad! It's true I'm burned out on open source but our exchanges so far have been rather low-stakes and invigorating — I don't mind further PRs, we should probably coordinate a tiny bit just to make sure we don't tackle the same h2spec cases (if that was in your plans).

FrankReh commented 1 year ago

Hah. It's hardly likely we are tackling the same issues. I'm just seeing your commits fly in and as I see/read them, there's another one following it up! Commits that are short, sometimes with TODOs for the next step, are easy for others to consume and are likely avoiding stepping on other work ideas.

I will say reading through your use of tokio-uring is interesting and I went so far as fork it and make the changes necessary to depend on the latest version, which is not tagged with a version yet. So when we get around to issuing a new release of it, I hope you'll accept my PR to change your dependency version on it.

I was starting to trace the use of the ServerDriver trait and the errors it accepts and how the errors are used by other parts of the code, so see about the returning 400 TODO, but I kept getting distracted by other interesting types that I wanted to understand better first. I wouldn't view any of my time wasted if you ended up fixing it all up yourself first. But if I do start actually writing code, I'll draft a PR showing it for sure.