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

setManualAcks in QoS 2 #994

Open
lacourte opened this issue Apr 27, 2023 · 2 comments
Open

setManualAcks in QoS 2 #994

lacourte opened this issue Apr 27, 2023 · 2 comments

Comments

@lacourte
Copy link

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

  • [x ] Bug exists Release Version 1.2.5 ( Master Branch)
  • [x ] Bug exists in MQTTv3 Client on Snapshot Version 1.2.6-SNAPSHOT (Develop Branch)
  • [x ] Bug exists in MQTTv5 Client on Snapshot Version 1.2.6-SNAPSHOT (Develop Branch)

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.

  • when the client receives a QoS 2 PUBLISH, it immediately sends the PUBREC reply message (ClientState.notifyReceivedMsg, line 1226)
  • when it receives the PUBREL, it immediately sends the PUBCOMP (ClientState.handleInboundPubRel, line 1151)
  • the messageArrivedComplete function sends a duplicate PUBCOMP (CommsCallback.messageArrivedComplete, line 502)

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 added a commit to lacourte/paho.mqtt.java that referenced this issue Apr 28, 2023
lacourte added a commit to lacourte/paho.mqtt.java that referenced this issue Apr 28, 2023
Signed-off-by: Serge Lacourte <[email protected]>
(cherry picked from commit b4f6945)
@lacourte
Copy link
Author

lacourte commented Sep 6, 2023

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
Copy link

whizyrel commented Aug 1, 2024

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.

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

No branches or pull requests

2 participants