`NettyTcpServer` as a `Publisher<Connection>`

[NiteshKant]

This issue is to discuss the current implementation of NettyTcpServer which is a Publisher<Connection>

The issue with this design is that there is no way to get a handle of the lifecycle of connection processing. There are various ways in which a server would like to manage the lifecycle of a connection processing:

• Cancel processing, in response to an out of band event like traffic shaping. I am assuming that this would naturally progress to an HTTP server being a Publisher<HttpRequest>. If so, then request cancellation is a first class usecase, for which we would need a handle to request processing, without closing the connection.
• Cancel processing on connection close: If a connection processing is async then a connection close would not trigger a cancellation unless a write/read is issued.
• Arguably one can delay processing by lazy subscriptions.

By modeling a server as a Publisher of T we can not get an async representation of the processing of T, since the notification is

public void onNext(T t)

[jbrisbin]

The Reactive Streams contract provides the same events in processing a connection as a dedicated abstraction would. It just uses different names. At the server level these would be:

• onSubscribe: a new server has been created and is being prepared for processing by the global pipeline. The implementation of Subscription being passed should be aware of any special processing needed to prepare (like configuring the Netty Bootstrap).
• onNext: a new connection has been made by a client. The Subscriber should handle the per-connection events.
• onError: the server has received a fatal error at the server level (as a result of bootstrapping, most likely) and will terminate.
• onComplete: the server has shut down and is exiting.

At a connection level, these events correspond to:

• onSubscribe: a new connection has been created and the equivalent of a ChannelInitializer is being activated and a sort of ChannelContext is being passed to the Subscriber in the form of a Subscription implementation.
• onNext: a new message was received by the transport implementation and is being passed to the handler.
• onError: a connection-level error has occurred and the handler is terminating abnormally.
• onComplete: a connection has been closed, either normally or abnormally.

[rstoyanchev]

As I mentioned under Issue 5 I see no reason why use a reactive streams Publisher for new connections. Can reactive back-pressure even be applied in this case? Connections arrive at the rate they will, it's not something we can push back on. It almost sounds like the intent is to do some kind of load balancing but I can't quite see how this is the right place to do that.

Also by making the server a Publisher isn't that basically saying that a server is parameterized to handle one protocol only?

I would much prefer to see a dedicated abstraction for protocol handling.

[NiteshKant]

@jbrisbin I was referring to not having a handle on the processing of a connection which can be cancelled if need be. onNext from a Publisher to a Subscriber can not give us a handle to that (essentially a Publisher<Void>) as onNext does not have a return value.

Can reactive back-pressure even be applied in this case?

Yes, one can stop accepting connections on a server by changing the selector interest sets. Netty provides this via turning off AutoRead on the server channel and issuing explicit reads whenever there is a need of accepting more connections.

Having said that, I don't think this is the correct abstraction for the reason I mentioned in this issue.

[jbrisbin]

@NiteshKant @rstoyanchev added two commits to change how connections are handled. The first implements write functionality and removes the Publisher<Connection> dependency. The second introduces some functional interfaces just to demonstrate how we might support either creating a new ConnectionHandler for every connection or using a singleton. We use this pattern frequently in Reactor and it's more flexible in many ways that requiring a reference to an instantiated object.

[NiteshKant]

added two commits to change how connections are handled.

May I suggest that we follow the pull-request model for changes in the repo? That makes it easier to follow changes :)

[NiteshKant]

Closing as we agreed on not representing a server as a Publisher in #24