search

eclipse / paho.mqtt.java

Eclipse Paho Java MQTT client library. Paho is an Eclipse IoT project.
https://eclipse.org/paho
Other
2.11k stars 883 forks source link

Initiating new connections is slow #651

Open sp193 opened 5 years ago

sp193 commented 5 years ago

Please fill out the form below before submitting, thank you!

Hi all,

I would like to highlight that Paho v1.2.1 seems to take quite a while to establish a single connection. I have only recently started using it in favour of the aging Fusesource MQTT client, but it seems to be considerably slower than v1.2.0 and the Fusesource MQTT client when it comes to establishing new connections.

I have only tried out the MQTTv3 client, as I am going through Apache Camel (which does not support a number of Paho's newer features). My project only requires MQTT v3 too.

My benchmark was the creation of 400 connections & subscribing to a topic each, through the creation of 400 Camel routes. Fusesource needed only about 8s, while Paho v1.2.0 needed about 9s. Starting from v1.2.1, it requires 140s to complete the same task.

I did a Wireshark capture and noted that there is a 250-300ms delay after the 3-way TCP handshake and the MQTT Connect command. At first glance, it seemed to be a problem with Nagle, due to 892ec11. But commits up to right before 4b23f12 do not seem to exhibit the same delays. Perhaps 4b23f12 has something to do with this.

Thank you for your attention, and have a nice day.

Paho v1.2.0: Imgur Paho v1.2.1: Imgur

sp193 commented 5 years ago

I found that the overhead was caused by the additional calls toThread.sleep(). By changing the way threads are started and stopped, Paho v1.2.1 can start 400 connections in 9034ms.

Why must we wait for a thread to start? Surely an exception would be thrown, should a thread fail to start. As for stopping, I did a Thread.join(), after getting a copy of the Thread object.

Please refer to this patch: If it is suitable, I can make a pull request. Thanks for looking at it!

edit: removed old link.

sp193 commented 5 years ago

I gave it some thought and perhaps the reason why it was preferable to wait for the thread to really start, is to ensure that the state machine will surely transit between states properly... not so much that the thread would fail to start. For example, if stop() is called immediately after start(), but yet the thread has yet to run. Then things like callbackThread would still be null, and stop() would fail to stop the callback thread.

start() should probably still wait for the thread to change the state machine to enter running state, which will allow the sendThread/recThread/callbackThread variables to be updated first.

Please refer to this updated commit instead:

EDIT: I found that my implementation has some problem as it appears to pause randomly. I have to attend to other matters at the moment, so this will have to wait.

sp193 commented 5 years ago

Hi again,

I think the pauses were caused by my network, as the lag does not occur when tested with a local MQTT broker. No errors were logged either. This has been tested on two independent machines.

Hence here is my patch: Once again, thanks for looking at it.

edit: removed old link.

sp193 commented 5 years ago

Hi again,

I was reviewing my changes and noticed that commit a6faf77 introduced a change that cancels the thread if ExecutorService is used. As a result, I have changed the design of my patch to always gracefully wait for callbackThread, recThread and sendThread to stop, regardless of whether ExecutorService is used or not. I also changed start() of these threads to always immediately enter Running state, to remove the need to synchronize with the new thread.

Please refer to this commit: https://github.com/sp193/paho.mqtt.java/commit/59fe086000905e8dd2154d6d2c12b991a869841d