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
Support for AutoAckOff #578
Conversation
Signed-off-by: shivam <shivamkm07@gmail.com>
Thanks for the PR - before I review it can you please sign the ECA (please let me know if you have any issues). Unfortunately I cannot accept a PR without a signed ECA (the aim being to ensure that all contributions are appropriately licensed). |
Done! |
Thanks very much for the PR. I have taken a quick look today and list a few concerns below; please note that these may seem fussy but I think its important that we get this right (once live it is very difficult to change the API and I want to minimise future issues caused by users not understanding how this all works). It's probably possible to address these concerns by documenting the My concerns are:
Adding this feature is likely to result in longer running handlers so I feel that its important to note:
Given these concerns I did consider whether we are going about this in the right way; alternatives would include:
Note: I'll copy this to the issue because it would be good to get feedback from users on this. |
Hi @MattBrittan and @shivamkm07, thanks for the PR! I've been running into a similar issue, and stumbled over the auto-ack. Coming from other message passing platforms, like Kafka and PubSub, I was very surprised that auto-ack'ing is even a thing (afaicr it's not even an option for Google's official PubSub library), especially since the Message.Ack() is essentially pointless because of that (at least to the end user of the library). My preferred option would be to have a ClientOption to at least turn it off (but ideally, not even auto-ack to begin with, but I understand this will create compatibility issues). I'm happy to give a PR for this a go if you two are busy. One crucial piece that MQTT brokers might be missing to make this really useful is some kind of backoff feature to delay re-delivering messages longer and longer if writes fail (so that we aren't spamming all of our server instances constantly with the same message upon failure), but that's beyond the scope of the client library I think. |
This reverts commit e28a1db.
Signed-off-by: shivam <shivamkm07@gmail.com>
Hi @MattBrittan and @Volcore, Thanks for reviewing the PR. I agree adding |
Signed-off-by: shivam <shivamkm07@gmail.com>
Signed-off-by: shivam <shivamkm07@gmail.com>
Thanks very much for implementing this (sorry it took a few attempts!). |
@MattBrittan Is there any expected timeline at which the next patch release is scheduled? That would be better than using the latest master commit. |
Done - I had been holding off because of the major changes to status handling but as they have been in @master for a couple of months without any issues being raised I felt it was worth making a release. |
Thanks @MattBrittan!!! |
This is to support disabling of Automatic Acking of messages by mqtt client. A similar solution has been merged in V5 client(eclipse/paho.golang@a47276e). While V5 solution also takes care of Acks being sent in order, this doesn't seem a necessity as discussed here.
Issue Resolved: #459