Add forward
#71
Add forward
#71
Conversation
wit/streams.wit
Outdated
| /// yet it does not change the ownership structure of them. I.e. they remain | ||
| /// children of their parent resources. If the parents place any lifetimes | ||
| /// restrictions on their children, those continue to apply. | ||
| forward: func(src: input-stream, dst: output-stream) -> future-forward-result; |
There was a problem hiding this comment.
These arguments will pass ownership of both the input-stream and output-stream. I believe sending an owned input-stream is correct here. In some use cases, we need a way to permit further use of the output-stream after forwarding has either finished or failed. We can do that by either making this parameter a borrow of the output-stream, or we get an owned output-stream in the resolution of future-forward-result.get
There was a problem hiding this comment.
Changing dst: output-stream from an owned handle to a borrowed handle will introduce multithreaded access to the stream. I can't see why one would want this as there's no way coordinate between the writes from different tasks/threads. From end-user and implementation complexity perspective I'd go for: (1) consume the stream and hand it back later, or: (2) document & enforce that while the forward is in progress any attempt to access the borrowed streams will trap.
That being said, the way I designed it, the host is guaranteed that no one else can touch the streams ever again. In my mind this is a "feature", as the host can then permanently fuse them together. This may enable additional optimizations not possible otherwise.
An example that triggered me to file this PR in the first place is: kTLS. Piping a TLS stream into a Socket stream could "automagically" offload TLS to the kernel (or even the NIC). Requiring the output-stream to remain usable after the input-stream has ended would prevent this optimization. To be clear, I haven't actually implemented this, so assign as much value to this use-case as you want.
Anyway, I can see merit in both designs. Maybe they can coexist?
There was a problem hiding this comment.
I hadn't thought about that case, but I have heard of some use cases for wanting to use the destination stream after the source stream is consumed. So, I guess they can both coexist. Are you up for introducing both variants in this PR?
wit/streams.wit
Outdated
| /// has already been taken before. | ||
| /// - `some(ok(value))`: when the future is ready with a value. This is | ||
| /// returned only once. | ||
| get: func() -> option<result<stream-error>>; |
There was a problem hiding this comment.
We also need this get function to have a way to represent "youve already retrieved the result of this and i cant give it to you again", in http we used an outer result for that, so it would end up like option<result<result<stream-error>, _>>
There was a problem hiding this comment.
Indeed, that's documented as:
- `some(error(_))`: when the future is resolved, but the result value has already been taken before.
There was a problem hiding this comment.
oh, sorry, I read the type signature before the docs here. How do we represent the case where the forward completed with success? there is no stream-error to represent success, you need a result<stream-error> for that.
There was a problem hiding this comment.
Well "success" in this case is when one of the streams ended with closed. Which is already captured in the stream-error type.
There was a problem hiding this comment.
OK, but as a user I would want to distinguish between whether the input-stream closed (expected outcome of the operation) and the output-stream closed (unexpected). When the expected outcome happens, we should return an ok.
Additionally I'd consider returning a variant forward-error { source(stream-error), destination(stream-error) } to distinguish where the error came from - we shouldnt throw away information that the user will need to debug.
There was a problem hiding this comment.
OK, but as a user I would want to distinguish between whether the input-stream closed (expected outcome of the operation) and the output-stream closed (unexpected). When the expected outcome happens, we should return an ok.
Fair enough.
Additionally I'd consider returning a variant forward-error { source(stream-error), destination(stream-error) } to distinguish where the error came from - we shouldnt throw away information that the user will need to debug.
I took this design from splice which already tosses errors from both streams on the same pile. Are you sure we want to introduce this inconsistency & complexity in return types? Or is it OK to say that whenever a consumer really cares about which stream failed, they can just attempt input-stream::read(0) or output-stream::check-write() after the fact?
|
I've updated the definitions. Pretty much the entire text has changed so please check again (sorry). Noteworty changes:
I have not added the |
|
After discussion in #73, I realized my interpretation of flush was not quite correct. I've removed the |
During the plumber's summit I noticed that a forwarding mechanism was up for consideration in preview2.1.
So, here's my take.