Extensible invocation of TcpClient in ReactorNettyTcpClient

[rupebac]

We would like to achieve something as basic as sending all Stomp sessions from a specific user to one Stomp relay, while other users may go to other Stomp relays. This should minimize I believe the traffic inside the Artemis cluster.

I know I can use TcpClient#remoteAddress(Supplier...) but this is not enough for what we try to achieve. We need to know the context of the incoming connection to know to which remoteAddr send that to. I mean, we need to gather the session id / user principals of the incoming session, to decide to which relay it should send that Connect request to.

I know I could extend ReactorNettyTcpClient and Override connect(TcpConnectionHandler) but then I need to reimplement myself the whole ReactorNettyHandler (as this one is private).

Ideally ReactorNettyTcpClient should internally allow some sort of tcpClient resolver, or at least it should, I think, use a getter for the TcpClient (so that I can override the getter myself).

Is this possible at all with the current code base? What would be then the correct way to achieve this?

[rstoyanchev]

If you override ReactorNettyTcpClient#connect you still don't have access to session or user id. That's pretty low level with no knowledge of the context in which the client is used. Come to think of it I'm not entirely sure what the context here is? Is it in the context of the StompBrokerRelayMessageHandler, where we could add a protected method to obtain the client to use also passing in the StompAccessor, or in the context of ReactorNettyTcpStompClient?

Also could clarify how this would minimize traffic inside the cluster?

[rupebac]

You mean, a protected method in StompBrokerRelayMessageHandler called by StompConnectionHandler#connect? That would work out yes. Also centralizing the access to the tcpClient in ReactorNettyTcpClient from both #connect methods in a protected getter would also make it.
Additionally, I have opened a PR, to populate the sessionId in the TcpClient attributes. Leaving more room for more customizations there in the future.
I guess the main problem is that all this code was thought on times where TcpClient did not have any lazy evaluation of the remote address, and you can still see the StompBrokerRelayMessageHandler#getRelayHost which does not make much of a sense with a lazy evaluation of the remoteAddress via TcpClient.

[rstoyanchev]

I meant turning this line into a protected method on StompBrokerRelayMessageHandler. The same could also be done for this line too with the ReconnectStrategy passed in as a @Nullable argument.

As for the PR I would prefer to go with the above for now.

[rupebac]

I do not see how what you are suggesting can solve the problem. I override that new method, and then what? I call the super and I just get the Mono, but I still won't have any control on deciding where to connect that specific session without writing some sort of pool of ReactorNettyTcpClient (each of them bound to a fixed stomp server). What we want to customize is the internal TcpClient inside ReactorNettyTcpClient to be aware of the session id, right ?

btw, I forgot to clarify the use case:

Tomcat 1 ---- post msg to user2 ----> Stomp Broker 1 ----> Stomp Broker 2 ----> Tomcat 2 ----> user2

So we basically want to make sure that all our tomcat servers in the cluster, when delivering a message to "user2" get all connected to where user 2 is connected. As otherwise there will be some internal traffic and lag happening. So the picture looks more like this:

Tomcat 1 ---- post msg to user2 ----> Stomp Broker 2 ----> Tomcat 2 ----> user2

You understand what I mean?

[rstoyanchev]

It would be something like this by default:

protected void connectHandler(StompHeaderAccessor accessor, 
        TcpConnectionHandler<byte[]> handler, @Nullable ReconnectStrategy strategy) {

    getTcpClient().connect(handler);
}

You would then override that and choose a differently customized client depending on the info in the STOMP headers.

[rupebac]

yes, that could kind of work, it is not super nice, since I will have to instantiate a ConnectionProvider, LoopResources and TcpClient for each of these tcpClient, but it will be assumable if you think this is an edge case. Hopefully another approach will arise in the future, with a more solid support for client side load balancing.

[rstoyanchev]

I've added something along the lines of your pull request but with a StompTcpConnectionHandler sub-interface which exposes the sessionId as well as the STOMP CONNECT headers.

[rupebac]

yes, I have checked your commit. Looks great, no objection. Thanks so much for your time.