Affects: 5.2.8.RELEASE

@Configuration
@EnableWebSocketMessageBroker
public class MyConfig implements WebSocketMessageBrokerConfigurer {

    @Override
    public void configureMessageBroker(MessageBrokerRegistry registry) {
        // ...
        registry.setPreservePublishOrder(true);
    }

    @Override
    public void configureWebSocketTransport(WebSocketTransportRegistration registry) {
        registry.setSendBufferSizeLimit(512 * 1024);
    }
}

The setSendBufferSizeLimit method has no more effect after setPreservePublishOrder(true).

The reason is that the instance of OrderedMessageSender keeps all messages and sends only one message at a time to the buffer.

Thus, the application can slow down significantly or even crash with OutOfMemory Error instead of just closing the WebSocket connection when the messages queue overflow.

Memory Analyzer shows how OrderedMessageSender can occupy the whole heap: Heap

Comment From: rstoyanchev

Indeed, sendBufferSizeLimit is at a much lower (transport) level while OrderedMessageSender buffers at the broker. Further the sendBufferSizeLimit is per session while OrderedMessageSender has one queue. That means one slow session/connection can force other sessions to wait and the queue to fill up.

Technically only messages within the same session need to be serialized, so we can explore an improvement where messages are held up only if there is another message from the same session already in progress. In addition we could also implement some sort of timeout at the level of OrderedMessageSender.

Comment From: rstoyanchev

@OleksiiKlochko I've made a change so that messages are processed one at a time but only within a session and not across all sessions. This should remove a head-of-line blocking where slow sessions affect others.

Would you be able to give this a try using 5.3 snapshots? It is a significant improvement of throughput and might be sufficient to address your issue.

Comment From: OleksiiKlochko

@rstoyanchev I dug deeper and found that we have one OrderedMessageSender per session. Please see https://github.com/spring-projects/spring-framework/blob/7288ae1c1613a73710eda53e8108d3c375f3b618/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/AbstractBrokerMessageHandler.java#L294-L302 I might be wrong but the current fix is not suitable.

I suppose that we should have the same sendBufferSizeLimit in OrderedMessageSender as for WebSocketTransportRegistration. This provides consistent behavior for ordered and unordered processing.

Thanks!

Comment From: rstoyanchev

Thanks for taking a look. Indeed I overlooked that OrderedMessageSender is a per-session wrapper around the singleton MessageChannel. I reverted the commits.

Even ff the same limits are replicated, the challenge is what to do when the limit is reached. A MessageChannel can only pass messages, it can't close the session for example.

I think we have to avoid waiting until the current message is fully processed which includes the time to send. For the purpose of ordering as soon as the message is queued up in ConcurrentWebSocketSessionDecorator we can release the next. That will allow the existing send limits be enforced. I'll experiment with how this can be done.

Comment From: rstoyanchev

I've committed a solution along the lines of my last comment. The next messages is now released when the previous is queued in the send buffer of ConcurrentWebSocketSessionDecorator before it is sent which means messages will accumulate there eventually violating the send limits. If you can give it a try to confirm that would be great.

Comment From: OleksiiKlochko

I've checked your solution, it works as you described. That's excellent! Many thanks for your help and involvement.

Comment From: rstoyanchev

Thanks again for taking the time to verify before the release, much appreciated!