eclipse / paho.mqtt.java

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

setManualAcks in QoS 2 #994

Open lacourte opened 1 year ago

lacourte commented 1 year ago

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

If this is a bug regarding the Android Service, please raise the bug here instead: https://github.com/eclipse/paho.mqtt.android/issues/new

The setManualAcks function is not implemented for QoS 2 messages.

The implementation for QoS 1 messages is fine. The PUBACK reply message is delayed until the client calls the messageArrivedComplete function. This somehow allows the client to commit the processing of the message. If anything fails during the processing, then the standard recovery of MQTT will deliver the QoS 1 message again.

The implementation for QoS 2 messages is incomplete, and I believe erroneous.

I believe that the manualAcks option should work for QoS 2 messages exactly as for QoS 1 messages. The PUBREC message (and not the PUBCOMP) should be delayed until the client calls the messageArrivedComplete function. This would allow the same commit like feature which is particularly useful in QoS 2. If anything fails during the processing, then the standard recovery of MQTT will deliver the QoS 2 message again. The function should not be fixed delaying the PUBCOMP message, as a specific recovery mechanism should then be implemented.

lacourte commented 11 months ago

There is a remaining bug related to the manual ack in QoS 2, that is not covered by the fix. This bug is effective in both versions of the client, v3 and v5.

The messageArrivedComplete function may be called by the client before it receives the PUBREL message. As a result the PUBCOMP message may occur before the PUBREL message, which violates the MQTT protocol. Beyond the violation itself, this may have unforeseen consequences in the broker and/or the client.

whizyrel commented 2 weeks ago

Hello guys, great job here but this issue is still persistent with v5 1.2.5. I have read #994, and #1023, it does not happen to have a fix. I have submitted this issue #1054.