Delete mqtt qos0 queue when mqtt 5.0 connection is closed #14006
Conversation
MarcialRosales
changed the title
MarcialRosales
changed the title
MarcialRosales
force-pushed
the
delete-qos0-queue
branch
2 times, most recently
from
3b2caee to
04a3aa5
Compare
MarcialRosales
force-pushed
the
delete-qos0-queue
branch
2 times, most recently
from
2b87f22 to
496b2d0
Compare
MarcialRosales
force-pushed
the
delete-qos0-queue
branch
from
496b2d0 to
57ef5ea
Compare
| zero_session_expiry_interval_disconnect_client(Config) -> | ||
| C = connect(?FUNCTION_NAME, Config, [{properties, #{'Session-Expiry-Interval' => 0}}]), |
There was a problem hiding this comment.
This test case doesn't test the fix you provided.
In fact this test case succeeds even without your fix.
Specifically the bug is that an MQTT QoS 0 queue is not deleted by the MQTT plugin if an MQTT 5.0 client connects with CleanStart=false and a SessionExpiryInterval of 0.
Therefore, this test case should use {clean_start, false}.
I recommend moving this test case into the v5_SUITE since the combination of these two settings is an MQTT 5.0 specific feature.
There was a problem hiding this comment.
Thanks @ansd I was following the 3 conditions in the docs https://www.rabbitmq.com/docs/mqtt#qos0-queue-type and it does not mention clean_start setting .. should it also be mentioned in the docs?
I started doing this test in v5_SUITE but then I found mqtt_shared_SUITE and I thought this suite would be the best place because both versions are tested ...
There was a problem hiding this comment.
should it also be mentioned in the docs?
No, the docs are correct.
If anything, we can delete the 1st condition since it's no longer relevant. This feature flag has become required: rabbitmq/rabbitmq-website#2284
There was a problem hiding this comment.
I am reading (http://www.steves-internet-guide.com/mqttv5-clean-start-clean-sessions-and-session-expiry/) that clean_start is used in conjunction with session expiry interval . why do you think the documentation should not mention clean_start in the conditions?
There was a problem hiding this comment.
why do you think the documentation should not mention clean_start in the conditions?
If no queue exists for an MQTT 5.0 client, and the client connects with a Session Expiry Interval of 0 subsequently subscribing with QoS 0, the MQTT plugin will create an MQTT QoS 0 queue for that client irrespective of whether the client connected with Clean Start 0 or 1.
|
|
||
| ok = emqtt:disconnect(C), | ||
| %% After terminating a clean session, we expect any session state to be cleaned up on the server. | ||
| timer:sleep(200), %% Give some time to clean up exclusive classic queue. |
There was a problem hiding this comment.
You copied this comment wrongly. It's not a classic queue that gets created, it's an MQTT QoS 0 queue.
|
To be extra clear: The bug was that an MQTT QoS 0 queue was not deleted by the MQTT plugin for an MQTT 5.0 client that connected with Clean Start 0 and Session Expiry Interval 0. |
because it is a feature exclusive of v5
|
I have addressed the comment and trailing whitespace feedback. |
(cherry picked from commit 69baf91)
(cherry picked from commit 24464a6)
Delete mqtt qos0 queue when mqtt 5.0 connection is closed (cherry picked from commit d8b3288)
(cherry picked from commit 24464a6)
|
Thank you for this fix @MarcialRosales ! |
Proposed Changes
Fix issue where an mqtt qos0 queue is left alive after its connection is closed. An mqtt 5.0 connection with Session Expiry Interval of 0 and clean start of 0 should be deleted when its connection is closed.
Types of Changes
What types of changes does your code introduce to this project?
Put an
xin the boxes that apply