StompBrokerRelay doesn't send heartbeats to broker when @MessageMapping handles received messages

[xak2000]

I discovered that StompBrokerRelayMessageHandler just forwards all messages from webscoket connection. This includes heartbeats from STOMP (over webscoket) clients (including WebSocketStompClient).

This works fine. But there is one caveat here. Not all STOMP messages from webscoket are handled by StompBrokerRelayMessageHandler. Some of them handled by SimpAnnotationMethodMessageHandler because their destination has prefix equal to prefix, that is set in MessageBrokerRegistry.setApplicationDestinationPrefixes.

For example:

    @Override
    public void configureMessageBroker(MessageBrokerRegistry registry) {
        registry.setApplicationDestinationPrefixes("/app");
        registry.enableStompBrokerRelay("/topic", "/queue");
    }

If WebSocketStompClient just sends a message every 1 second to /app/greeting destination, then this message is handled by controller @MessageMapping("/greeting") and if that controler doesn't forward it to /topic/greeting in the same tcp-session (with RabbitMQ broker) (i.e. returning a value from controller's method), then RabbitMQ never receives that message.

It is common situation, I think. Not all application controllers should send something into broker. Especially into the same tcp connection that associated with websocket, from which message received. For example, I want to send message from my controller, but I want to send it through _system_ connection (not through connection, associated with webscoket, from which the message come) to the broker using SimpMessagingTemplate.

And the problem here is that WebSocketStompClient thinks it sends some bytes (it actually does), so it's org.springframework.messaging.simp.stomp.DefaultStompSession.WriteInactivityTask is never called.

But at the same time these bytes are never delivered to RabbitMQ broker, because they are handled by @MessageMapping controller. So the RabbitMQ doesn't receive any bytes in the heartbeat interval and closes the connection.

 [warning] <0.20898.54> STOMP detected missed client heartbeat(s) on connection 192.168.250.203:41574 -> 172.17.0.2:61613, closing it

I think there are two solutions here:

1. Just always send heartbeats from WebSocketStompClient on specified interval, without relying on whether some bytes were send recently or not.

   As I see, stompjs does exactly that. It is not exactly how heartbeats should work in STOMP protocol as described here, but it is more defensive (and produces more traffic of course).

2. Do not forward heartbeats from WebSocketStompClient (and any other connected websocket STOMP client), but do heartbeating with broker internally instead, like it does for _system_ connection. At the same time, do not forward heartbeats from broker to the webscoket, and instead do heartbeating with websocket internally. So, there will be two independent sets of heartbeat senders/chekers: one to broker side, and one to websocket side. So, even if the client will send only application messages (and will not send heartbeats because of that), the broker will still receive heartbeats from internal scheduler and will not close the connection.

   This soulution is more server resource intensive I think. The relay will have to do some scheduling instead of just relaying heartbeats back and forward, but it looks like a more "right" solution, that doesn't violate STOMP protocol (in the sense of not sending heartbeats to the broker in specified interval because of @MessageMapping).

   While I think this solution is more "right", I'm worried about performance here. For example, to service 10k webscoket connections, it should be 10k onWriteInactivity and 10k onReadInactivity checks on both sides , total 40k.

[xak2000]

Actually, maybe there is third way to solve this. Or rather slight variation of my second variant above (but using much less scheduling tasks, equal to websocket connections count).

Because all messages from the broker to the server are always forwarded to the client (Am I right?), we can just forward heartbeats from the broker to the client and let the client detect lost heartbeats, as it's done now.

It is only heartbeats from the client to the broker are problematic.

So we can use con.onWriteInactivity for broker connections as it's done for _system_ connection, but with slightly more sofisticated handler logic.

When any message is received from Webscoket, the current time should be written into lastReadFromWebsocket variable (it must be updated on receiving of any message, including routed to @MessageMapping - it just means that websocket is still alive). Then, when onWriteInactivity handler is triggered, we check how much time is passed from lastReadFromWebsocket till now. If it is within the heartbeat interval, we manually send a heartbeat into broker connection, else we do nothing (and let the broker to disconnect us if it wants to) as it means that the client doesn't send any message to us (routed to broker or not) in the heartbeat interval, so we also shouldn't send a heartbeat to the broker.

This way:

• If the client doesn't send any messages in a heartbeat interval, it will send a heartbeat to the server and this heartbeat will be routed to the broker, so onWriteInactivity handler will not trigger.

• If the client sends only @MessageMapping messages in a heartbeat interval, it will not send a heartbeat (because it thinks it already sent some traffic - it doesn't know that this traffic is handled by @MessageMapping and doesn't reach the broker) and onWriteInactivity handler will trigger. This handler will see, that last received message from Webscoket was not too long ago, so it will send a heartbeat to the broker itself.

• If the client doesn't send any messages and even heartbeats in a heartbeat interval, onWriteInactivity handler will trigger. It will see, that last received message from Webscoket was too long ago, and will do nothing. The broker has it's rights to treat that connection as dead and close it.

This can lead to more heartbeats than required to the broker. In worst case it doubles the heartbeat count. For example, if heartbeat interval is 60sec, and client send a message (handled by @MessageMapping) to the server 5 seconds after last triggered brokerCon.onWriteInactivity, then our server will send a heartbeat on next onWriteInactivity (55 seconds later), and the client will send a heartbeat 5 seconds after (60 seconds from last client message, 65 seconds after first onWriteInactivity). So, the broker will receive 2 heartbeats in 5 seconds (one forwarded from client and one from relay server).

This behaviour is totally unnecessary for clients, that sends heartbeats on fixed interval (like sockjs does), and will help only with clients, that sends heartbeats only after interval of write inactivity (like WebSocketStompClient does). So, to prevent unnecessary scheduling and excessive heartbeating, it can be controlled by some flag.

[rstoyanchev]

This is an issue indeed. Arguably we should do something on the server side to ensure client-to-server heartbeats aren't susceptible to such an issue. The StompBrokerRelayMessageHandler as well as the SimpleBrokerMessageHandler see all messages, but simply ignore some, so theoretically they are aware of the time of the most recent client message, so some such solution with an onWriteInactivity handler could work.

On the other hand I like the simplicity of changing WebSocketStompClient to always emit like stomp.js or alternatively to be configured with some filter for which messages (e.g. "/app") to ignore for tracking heartbeats.

[xak2000]

The problem with second option (fixing on WebSocketStompClient side) is that some other stomp client can still encounter problems working with Spring Stomp Relay support and that will be not client's fault, because that client will not violate STOMP protocol (it sends messages all the time, it just doesn't know that Spring doesn't resend some of them to the Broker).

I not checked all possible stomp clients and can't say how many of them working like this, but here is what I checked:

• stomp.js sends heartbeats on fixed interval
• Spring's WebSocketStompClient takes into account last sent frame time
• I searched "Android stomp websocket client" and the client from first google result uses exactly the same aborting mechanism as Spring's WebSocketStompClient.
• iOS clients that I checked doesn't send any heartbeats at all... :-)

[zerototwo]

spring-projects/spring-framework]

发自我的 iPhone

在 2019年6月29日，09:13，Ruslan Stelmachenko notifications@github.com<mailto:notifications@github.com> 写道：

The problem with second option (fixing on WebSocketStompClient side) is that some other stomp client can still encounter problems working with Spring Stomp Relay support and that will be not client's fault, because that client will not violate STOMP protocol (it sends messages all the time, it just doesn't know that Spring doesn't resend some of them to the Broker).

I not checked all possible stomp clients and can't say how many of them working like this, but here is what I checked:

• stomp.js sends heartbeats on fixed interval
• Spring's WebSocketStompClient takes into account last send frame time
• I searched "Android stomp websocket client" and the clienthttps://github.com/NaikSoftware/StompProtocolAndroid from first google result uses exactly the same aborting mechanismhttps://github.com/NaikSoftware/StompProtocolAndroid/blob/e71248f169682e528302111ab276174263dfa278/lib/src/main/java/ua/naiksoftware/stomp/HeartBeatTask.java#L63 as Spring's WebSocketStompClient.
• iOS clients that I checked doesn't send any heartbeats at all... :-)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/spring-projects/spring-framework/issues/22822?email_source=notifications&email_token=AJ4OKJNSYVFLJUB3MC6C3C3P42ZMFA5CNFSM4HHJ6DOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3OPZY#issuecomment-506914791, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AJ4OKJJALQMTTOGUJZGVLLTP42ZMFANCNFSM4HHJ6DOA.

[gludington]

In terms of modifying the client side to emit always in the same mold as stompjs, can we add a public heartbeat method to StompSession/DefaultStompSession? heartbeat messages passed to DefaultStompSession#send will fail the destination check Assert statement, and getting directly at DefaultStompSession#execute or the underlying connection in the WebsocketStompClient requires reflection, which is arguably more difficult/brittle than attempting something on the server side.

Edit: or by making a subclass/custom implementation of StompSession with additional method, and tying to that implementation.

[c0ntradicti0n]

Are there any news to this after one year or is there any known solution?

[petros94]

Any news on this issue?

[kmandalas]

@spring-issuemaster @xak2000 IMHO this is not an enhancement but a critical bug. I run into it recently and I ended up on this issue while searching. The WebSocketClient sends heartbeats when idle but when there are messages exchanged i.e. actual business functionality is taking place, then we get disconnections! As a temporary workaround in my project we always send a dummy empty message to a "/topic/dummy" Topic from the WebSocketClient, each time we get an actual (business) message from the server.

[rstoyanchev]

I think there is a reasonably straight-forward solution to this. As mentioned earlier StompBrokerRelayMessageHandler and SimpleBrokerMessageHandler do see all messages but ignore some by the destination and we can take actions for such ignored messages too. In fact SimpleBrokerMessageHandler already calls updateSessionReadTime before it ignores any message which means it does not have this issue. For StompBrokerRelayMessageHandler we can maintain a similar per-session timestamp and the timestamp indicates we should send something to the broker we can send a heartbeat if we receive a message that is ignored.

[rstoyanchev]

In the final solution, we keep a count of messages in the current heartbeat period for each connection. When the count is 0 and a message to a non-broker destination is ignored, a heartbeat is forwarded instead. To enable this a TaskScheduler needs to be set through the StompBrokerRelayRegistration. This keeps a task running to clear the message counts for each connection at the start of its heartbeat period.

[sharmilay2k]

@rstoyanchev @spring-issuemaster We have a similar issue, when we use Gatling to simulate web socket clients for our performance testing. We wanted to keep the web socket connections alive for say 1 hr. But unlike sockJS, Gatling clients don't have an option to send heartbeats and connections are getting timed out. We tried to send dummy messages to application using @MessageMapping but still tcp connection gets timeout as these messages does not reach broker. Looks like the above fix can solve the issue. I wanted to give a try to see how it behaves with 5.3.0 but I noticed setTaskScheduler()is added as a void method. I have another void method to be called to set the tcp connection setTcpClient(). Not sure how to set both together.

brokerRegistry
.enableStompBrokerRelay(DESTINATION_PREFIX_TOPIC)
.setClientLogin(clientLogin) .setClientPasscode(clientPasscode) .setSystemLogin(systemLogin) .setSystemPasscode(systemPasscode) // returns StompBrokerRelayRegistration .setTaskScheduler(new DefaultManagedTaskScheduler()) // returns void .setTcpClient(createTcpClient()); // returns void

We use Artemis as External Broker.

[rstoyanchev]

Thanks for catching this. I've created a follow-up issue #26049 to fix it. By the way you can also send heartbeats to the server which are simple frames with just a newline. Also I'm not sure if you've considered using Spring's STOMP/WebSocket client for the tests which does support heartbeats.

[sharmilay2k]

Thanks for the quick acknowledge @rstoyanchev . Yes we tried sending [\"SEND\\ndestination:/actions/ping\\nsimpMessageType:HEARTBEAT\\nheart-beat:120000,120000\\nid:0\\nreceipt:0\\ncontent-type:text/plain\\ncontent-length:1\\nbody:\\n\\n\\n\\u0000\"] frame through @MessageMapping which in turn publishes it to a dummy topic /topic/ping.

But I still see the error
Received ERROR {message=[AMQ229014: Did not receive data from /172.27.163.104:58546 within the 240,000ms connection TTL. The connection will now be closed.]}

I still don't understand it clearly, will this frame reach the broker and if I get above error message does that means the connection is closed really on the broker side?

[rstoyanchev]

You can't send them from @MessageMapping methods. Those use a shared connection. You have to send them from the client on the client connection.

[sharmilay2k]

Sorry for the very late reply. Could you please suggest how can I send a message through client connection. I have sent the SENDframe to a destination /actions/ping which is handled by a @MessageMapping annotation (I thought this will send the message through client connection from the client).

[sharmilay2k]

@rstoyanchev Any suggestions on above?

[jikuanyu]

I also encountered the same problem, but it was not resolved

[jikuanyu]

I use old version, 5.1.x，how do?