Skip to content

NettyTcpServer as a Publisher<Connection> #7

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

Closed
NiteshKant opened this issue Mar 12, 2015 · 6 comments
Closed

NettyTcpServer as a Publisher<Connection> #7

NiteshKant opened this issue Mar 12, 2015 · 6 comments

Comments

@NiteshKant
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

@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
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants