-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
PUBLISH packet should NOT update the outgoing queue #30
PUBLISH packet should NOT update the outgoing queue #30
Conversation
Broker uses `brokerCounter` to track how many packets it emits, as `messageId` will be rotated after it reaches max 65535. If there are two PUBLISH packets which brokerCounter is different but same messageId, the second PUBLISH will update the first slot in the queue, as `outgoingUpdate` is using `for` loop starting at index 0. This will fix the issue. We use `outgoingEnqueue` or `outgoingEnqueueCombi` to enqueue PUBLISH packets, and and use `outgoingUpdate` to update those PUBLSIH items to next state (say PUBREL), so `outgoing` array should not contain PUBLISH, and that's why this issue could be fixed by a check `packet.cmd !== 'publish'`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this going to cause the other persistences to fail?
What other persistences do you want to check? |
aedes-persistence-mongodb https://github.com/mcollina/aedes-persistence-mongodb aedes-persistence-redis https://github.com/mcollina/aedes-persistence-redis aedes-cached-persistence https://github.com/mcollina/aedes-cached-persistence They all treat aedes-persistence functions as interfaces, and implemented by themselves. Quite confident it won't affect those. |
what's the difference between this and #31? |
Just wanna submit a small change for ease to have a review. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update aedes-packet as well? I'm going to bump a major.
Can we close this and I review #31 instead? |
sure. |
Broker uses
brokerCounter
to track how many packets it emits, asmessageId
will be rotated after it reaches max 65535. If there are two PUBLISH packets which brokerCounter is different but same messageId, the second PUBLISH will update the first slot in the queue, asoutgoingUpdate
is usingfor
loop starting at index 0.We use
outgoingEnqueue
oroutgoingEnqueueCombi
to enqueue PUBLISH packets, and and useoutgoingUpdate
to update those PUBLSIH items to next state (say PUBREL), sooutgoing
array should not contain PUBLISH