-
Notifications
You must be signed in to change notification settings - Fork 179
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
When an MQTT connection dies, it fails with a NPE and never reconnects #1181
When an MQTT connection dies, it fails with a NPE and never reconnects #1181
Comments
The Vert.x MQTT client does not support reconnection, unfortunately. |
That would mean that smallrye must provide this part. Would you accept PRs fixing that in smallrye? Because right now, that would make the feature completely unusable from my point of view. Never recovering and not reporting this condition wouldn't allow any serious usage of the feature. |
Yes, sure, even if last time I checked it was quite complicated and should probably be something to add to the MQTT client directly. |
It looks like a PR for this already exists: #1081 Also it looks to me as if adding a functionality like this is too opinionated for vertx: vert-x3/vertx-mqtt#66 My proposal would be focus on the PR mentioned above. It looks like we are not the only ones having interest in this feature. |
Well, yes, there is another PR and good reasons while it's not merged yet. A quick run revealed a hidden thread pool and the reactive API is just delegating blocking calls on that thread pool. So, not really in line with the overall architecture. The outcome of vert-x3/vertx-mqtt#66 does not seem to be valid. I've discussed it with @vietj about it, and it's definitely a feature the client should provide. Unfortunately, at the moment, MQTT is not a priority. |
Implementing such a feature isn't a big (technical) problem. It only looks to me like people ask for it, so do we. I think I can find the time do the implementation. But I would really like to focus on the technical aspect then, and not go into lengthy discussions on the "why" and "where". So if you can tell me "where" and "how" you would want to see this fixed, then I can try to get it done as soon as I can. |
That would be awesome! My first guess would be to fix it in the VErt.x MQTT Client (in https://github.com/vert-x3/vertx-mqtt/blob/master/src/main/java/io/vertx/mqtt/impl/MqttClientImpl.java), Around https://github.com/vert-x3/vertx-mqtt/blob/master/src/main/java/io/vertx/mqtt/impl/MqttClientImpl.java#L260, you get a socket, and you may be able to attach a disconnect handler or just reuse the closeHandler: |
@cescoffier it's totally wrong. The MQTT client HAS TO implement MQTT 3.1.1 specification and not other unrelated stuff. The rest has to be at higher level so at SmallRye level or application level. |
Well, it needs to be somewhere. We got many requests for such a feature, even a fork of the connector using a different client that is handling the reconnect. Note that if the Vert.x client would have offered an easy way to detect connection failures and reconnect (as Paho is doing BTW), we would not need this new abstraction, and everything could have been done in the connector. Now if you want to keep the Vert.x client a pure implementation, we can:
|
I think handling reconnects is something that is not specific to smallrye, but others using MQTT as well. And other clients (like HiveMQ, Paho and Fuse) do it the same way. I am not sure why Vertx must be special here. So, if that it is a huge problem for everyone involved to put it into vertx-mqtt, then I would suggest create a vertx-mqtt-reconnect-client project (or some other name) with the content of the current PR. |
Regarding Paho having the reconnect feature, with a quick look on the code I noticed that it's just about reconnecting and then calling back to allow the higher level to add some logic. It's not trying to trace the subscriptions as we are trying to do with the PR on the Vert.x client. Anyway, If I am wrong please let me know maybe I missed some part of the Paho code.
I would not use the "huge problem" term, just highlighting that it's not MQTT part spec. Tbh I like more the solution of a specific project for it. Anyway I would pay attention to call it "Session", because it's ambiguos with what a session is in the MQTT land. |
…ession" This is based on the content of PR vert-x3/vertx-mqtt#197
This is based on the content of PR vert-x3/vertx-mqtt#197 Fixes smallrye#1181
This is based on the content of PR vert-x3/vertx-mqtt#197 Fixes smallrye#1181
This is based on the content of PR vert-x3/vertx-mqtt#197 Fixes smallrye#1181
This is based on the content of PR vert-x3/vertx-mqtt#197 Fixes smallrye#1181
This is based on the content of PR vert-x3/vertx-mqtt#197 Fixes smallrye#1181
Assuming there is successfully connected MQTT connection, which then just dies. In the log you can see a NPE (`NullPointerException') and the client never reconnects.
Full exception stack trace
The text was updated successfully, but these errors were encountered: