Skip to content
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

Avoid HIGH CPU when Peer disconnected #77

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

li-yanqing
Copy link

No description provided.

@fusesource-ci
Copy link

Can one of the admins verify this patch?

@ranl
Copy link

ranl commented Mar 2, 2017

@li-yanqing I'm not a maintainer of this repo but seems like this is related to an issue we are having as well. when our MQTT servers are restarted (in order to deploy a new version) all our MQTT clients experience high cpu utilization and only after we are restarting the clients the issue is resolved.

can you provide more context on how you came up with this fix and whether it solved your original problem?
thanks!

@li-yanqing
Copy link
Author

@ranl Thanks for sharing your problem. Actually, I have not been using this lib for a long time. I don't think it's a suitable library of mqtt client with good performance. There's no serious problem if you just use it for function testing except performance testing. I suggest another library netty-mqtt or my fork lyq/netty-mqtt

Okay, Let's talk about your problem. I can not remember all of it clearly. As I know, I add that codes for notify my program to stop sending message in "listener.onDisconnected()". At the same time, to invoke "transport.stop()" to stop transport(I don't know its mechanism very well, Maybe the transport use something like spin lock and waste lots of CPU after disconnection) .

@ranl
Copy link

ranl commented Mar 3, 2017

thanks for the reply @li-yanqing, also I was wondering y you made the send() method synchronized. can you explain the reason behind that?

also looking at this it seems that not configuring the reconnectAttemptsMax member (which is the default) makes this code kind of an endless loop since we will try to reconnect forever.

@li-yanqing
Copy link
Author

I make the send method as synchronized cause I found it lost some messages when lots of messages were being sent. Also that's why I gave up this library, In concurrency, counter is not very correct. I use "synchronized" for this method just want to keep correction of counter with reducing some performance. (Maybe you can use other locks to improve a little bit performance).

A big problem is that a client must use two or three threads to handle its business. If there're 60,000 clients in your testing, Any OS can not handle so many threads in a process. So I don't think it's a good idea to use this library in performance testing.

With another library(with Netty/Async), 60,000 clients had been simulated in my testing.

@ranl
Copy link

ranl commented Mar 6, 2017

thanks for the detailed response @li-yanqing ! I'll update in case we find something interesting.

@ranl
Copy link

ranl commented Mar 6, 2017

which library are you currently working with? can u share a link?

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

Successfully merging this pull request may close these issues.

3 participants