swift-nio icon indicating copy to clipboard operation
swift-nio copied to clipboard

Add a pipeline builder that checks the validity of the order of handlers

Open Joannis opened this issue 2 years ago • 13 comments

Add a pipeline builder that checks the order of handlers

Motivation:

While NIO's pipelines provide an exceptionally good framework for networking, there are two main outstanding imperfections in NIO's pipelines that are yet to be improved upon.

First of all, channels depend on NIOAny for unwrapping types, the pipeline handlers types are therefore not checked at compile time, leading to headaches - in particular when working with more complex Channel setups.

In addition to the above, channels require a fair amount of 'extra' calls to glue handlers together. After a handler received a read or write, it applies any logic, and writes their 'Out' type into the next handler in line. To call the next handler, handlers need to invoke a read/write on a 'Context' type which has knowledge of the pipeline. This context then calls the next handler with the same payload. This is suboptimal for the performance of NIO based networking.

While this PR doesn't fix either of those, it _does_reduce the headaches originating from a lack of type-checking. This PR would pave the way for the above two changes to be made in follow-up PRs. Not only does this improve the performance of SwiftNIO. It also improves the maintainability of SwiftNIO based applications significantly, while lowering the entry-barrier for newer network application developers.

Another future direction I can see this leading towards, is the addition of groups of handlers (as Johannes called them 'Fuses'). A 'fuse' could represent an HTTP1 or HTTP2 pipeline. From that change onwards, an HTTPS server would simply add the TLS + SNI handlers on a pipeline. Following the result of SNI, either the HTTP1 or the HTTP2 fuse would be appended to the pipeline, from then on handling the decrypted traffic.

Modifications:

This PR introduces a new ChannelPipelineBuilder, a result builder that uses buildPartialBlock's reducer pattern to check Inbound and Outbound types in both directions. This resultBuilder contains a special case allowing IOData and ByteBuffer to be used interchangeably.

Result:

After these changes, the result builder can be leverages to create a pipeline that is type checked. Immediately following the merge of this change, this will increase maintainability of NIO's pipelines.

This could in future versions lead to the elimination of the extra 'glue' calls within NIO itself. In addition to the above, it can be leveraged to provide APIs on Channels that are aware of the type(s) that can be written to a Channel, based on the type information available from this resultBuilder.

Joannis avatar Dec 17 '22 23:12 Joannis

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

Can one of the admins verify this patch?

swift-server-bot avatar Dec 17 '22 23:12 swift-server-bot

@swift-server-bot add to allowlist

Lukasa avatar Mar 02 '23 10:03 Lukasa

I have been playing with result builder based pipelines as well but I took it one step further where I actually changed the Channel protocol to also require to be fully typed and made the whole pipeline immutable. This worked out quite nicely and allowed to have fully typed pipelines that also supported protocol negotiation etc; however, this was all obviously API breaking and is something we can only consider doing for the next major.

The reason why I looked into this was to solve the protocol negotiation problem that we are currently facing in our design of the NIOAsyncChannel. We need to add the appropriate handlers of the NIOAsyncChannel to the pipeline once all protocol negotiation is done but we still have to do this synchronously otherwise we might lose data.

This PR is adding convenience on top of the current pipeline building to make sure we are not assembling a pipeline with mismatched types but it can still easily be broken with the current mutability. This still adds value but I would like to explore the design channels of NIOAsyncChannel + protocol negotiation with our current APIs first before we introduce these builders to make sure they fit together. So let's make some progress on the NIOAsyncChannel stuff and then we can circle back to this PR and see how it slots in.

FranzBusch avatar Mar 02 '23 13:03 FranzBusch