-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add UnsubmittedRead and Link API #294
Conversation
User can specify submission entry flags (e.g. FIXED_FILE, IO_LINK, IO_HARDLINK) via new set_flags().
THere are some API issue to address here. Opening up the flags to the user is presents the user with the ability to break the tokio-uring lifecycle managment. Setting The link API is also flawed. If a IOSQE_IO_LINK is observed in the final SQE at submission, it is ignored. The current API doesn't protect the user from that, or provide any way of knowing if the ring has been submitted in between the ops Finally, what stops the user experiencing concurrency errors, if await is called between read1.submit and read2.submit? Another task could submit to the ring. |
@ollie-etl Thanks for the review, you're absolutely right for possible edge cases. I myself tried to provide additional API while hiding the If possible (and viable), I'd like to discuss some of API options in this PR so complete this functionality in here. Let me leave some approches to discuss that direction in advance, I added let read1 = file.read();
let write1 = file.write();
let (res1, res2) = link!(read1, write1); This will expand as you can easily expect let read1 = read1.submit().set_flags(Flags::IO_LINK);
let write1 = write1.submit();
(read1.await, write1.await) With those API, we can hide uring primitive to users and prevent the mistakes. But I felt it's not very flexible and restricted because user cannot use futures directly. (e.g. what if I want to await only last one?, what if I just want to use existed API like (For other directions, I failed all the non-macro approaches as Any suggestion would be appreicated. |
Currently, there's no unsubmitted API change in read part yet. Add the new API following write part. Add simple link operation test cases to use read/write submittable operations.
My thinking on this is that I guess there is a question around Futures behaviour. Should the Link struct return a single future on submission? There are options here - do you only return once the chain completes, or do you provide futres for all the ops in the link chain? Both are varid, so we probably want to support both. |
I also tried to introduce somehow nested struct But the problem about Still, I want to try the direction as it's far cleaner API so leave here my trials briefly hoping better idea. struct UnsubmittedOneshot<D> {
fn link<D2>(self, other: UnsubmittedOneShot<D2>) -> Link<D, D2> {
// returns Link struct which have two different OneShot(s). It's gonna happen recursively.
}
}
struct Link<D1, D2> {
// Somehow awesome nested struct which have all operations.
fn submit(&self) -> MultiInflightOneshot<D1, D2> {
// Return new InflightOneshot which have several OPs.
}
}
impl Future for MultiInflightOneshot<D1, D2> {
type Output = ??? # This is dynamically decided (D1, D2, D3, D4... etc)
} |
struct UnsubmittedOneshot<D> {
fn link<D2>(self, other: UnsubmittedOneShot<D2>) -> Link<D, D2> {
// returns Link struct which have two different OneShot(s). It's gonna happen recursively.
}
}
struct Link<D1, D2> {
// Somehow awesome nested struct which have all operations.
fn submit(&self) -> InflightOneshot<Link<D1,D2> {
// Use the existing Inflight oneshot
}
}
impl Future for InflightOneshot<Link<D1, D2>> {
// Link Future returns a tuple of the output of the first operation, and a future for subsequent linked ops
type Output = (<D1 as Future>::Output, D2)
} |
let links = file.read().chain(file.write());
let (read_output, write_future) = links.await;
let (write_output, next_future) = write_future.await; You are suggesting such API above, right? It may be possible but not sure with the current API ( Let me try with some more modification. |
@ileixe yes, thats what I had in mind, although I haven't fully thought through all the implications / issues |
@ollie-etl I added I don't like introducing |
048cd07
to
ff8e180
Compare
@ileixe I'd like the opinion of @Noah-Kennedy, as this is building on a lot of his work |
This PR introduces two API changes:
UnsubmittedRead
in read part,Submit
andLink
struct.(1) is basically following suggested API change in: #244 and (2) is for linked operations.
User now can link their operations like below
Closes: #289