-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
Aedes stops responding after 99 messages when a client has overlapping subscriptions #88
Comments
@cordovapolymer is right. You might have to bump the concurrency value to something according to your requirement. |
Thanks for confirming it and testing against concurrency. If I get time I will try to debug it this weekend. |
@GavinDmello the issue is that, once it hits the concurrency limit, it hangs completely. The correct way of operation when concurrency limit was hit, is to deliver queued messages and then continue accepting new. |
@cordovapolymer Have you tried {concurrency: 0} |
@GavinDmello , of course it will lead to a leak, as we can see from @yutter test once setMaxListeners limit is hit, it stops accepting new messages and it doesn't flush already sent ones or resets count. |
@mcollina @GavinDmello @pkinney @dpatekar @eladnava @renatoc @behrad @ovhemert @ralphtheninja @wuhkuh |
@cordovapolymer Can you confirm that the publish events don't seem to work either? These start working once I reset the server, where it will throw a large chunk of missing packets. I might have more time to look into this bug later this week. It's actually quite interesting at first sight. |
What do you mean by resetting the server? When I restart the server, or restart the publisher(client2.js in the @yutter example, I've changed it to publish at 1msec rate) it recovers to hang again in a same way, if I restart client1 it accepts its connection then lags for about a minute and sends a bunch of packets without interruption and hangs again. In the case it's restarted when it's receiving qos 2 packets, it throws |
@cordovapolymer Have you tried the solution which I mentioned in the PR with { concurrency : 0 } |
@cordovapolymer @GavinDmello So here's what I'm interested in:
So a logical next step would be to check if the queue works properly. I have a hunch that the problem starts there. I'm currently on my phone and therefore unable to test this. You could continue my research so we can get to a working solution sooner rather than later. |
The issue is not the listeners , (atleast in this case it isn't, we might need more that the default listeners for drain). |
@GavinDmello |
@GavinDmello yes, try it yourself with @yutter demo, you can see it doesn't work. Also I've reviewed your latest PR. |
@cordovapolymer More on this issue later today. |
Yes, this bug is in MQEmitter. I will send a PR over to the MQEmitter package. There's more to fix; events in particular, so it's not a production-ready branch yet. |
@wuhkuh Nice catch |
Aedes is fast because it respect backpressure, e.g. it does not accept new messages if its internal queue is full. But that means we should clear it in any case. It seems there is a problem with overlapping subscriptions where things are not properly cleared. |
Yeah, only to find out this is intended. The queue has to be emptied on aedes' side. |
If a create an Aedes broker and have two clients, one publishing and one subscribing and the subscribing client has overlapping subscriptions such as:
client.subscribe('000001/021/#');
client.subscribe('000001/021/000B/0001/01');
the hub stops responding after 99 messages.
This is not the case when I use mosca as the broker. I have attached my test files
Archive.zip
The text was updated successfully, but these errors were encountered: