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

Store MQTT messages as non-durable if QoS 0 #11012

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Conversation

ansd
Copy link
Member

@ansd ansd commented Apr 16, 2024

By default, when the 'durable' message container (mc) annotation is unset, messages are interpreted to be durable.

Prior to this commit, MQTT messages that were sent with QoS 0 were stored durably in classic queues.
This commit takes the same approach for mc_mqtt as for mc_amqpl and mc_amqp: If the message is durable, the durable mc annotation will not be set. If the message is non-durable, the durable mc annotation will be set to false.

By default, when the 'durable' message container (mc) annotation is unset,
messages are interpreted to be durable.

Prior to this commit, MQTT messages that were sent with QoS 0 were
stored durably in classic queues.
This commit takes the same approach for mc_mqtt as for mc_amqpl and mc_amqp:
If the message is durable, the durable mc annotation will not be set.
If the message is non-durable, the durable mc annotation will be set to false.
@ansd ansd merged commit e96125b into main Apr 16, 2024
17 of 19 checks passed
@ansd ansd deleted the mc-mqtt-annotation-durable branch April 16, 2024 09:43
@michaelklishin
Copy link
Member

@ansd @kjnilsson is this something that we should mention in release notes or is it a matter of properly using the internal message container format?

@ansd
Copy link
Member Author

ansd commented Apr 16, 2024

We should mention in the release notes that, in 3.13.0 and 3.13.1, MQTT QoS 0 messages that were sent to classic queues were always persisted to disk which is unnecessary and may result in poorer performance. This is fixed in 3.13.2.

@ansd ansd added this to the 3.13.2 milestone Apr 21, 2024
ansd added a commit that referenced this pull request Apr 22, 2024
This is a follow up to #11012

 ## What?
For incoming MQTT messages, always set the `durable` message container
annotation.

 ## Why?
Even though defaulting to `durable=true` when no durable annotation is
set, as prior to this commit, is good enough, explicitly setting the
durable annotation makes the code a bit more future proof and
maintainable going forward in 4.0 where we will rely more on the durable
annotation because AMQP 1.0 message headers will be omitted in classic
and quorum queues (see
#10964)

For MQTT messages, it's important to know whether the message was
published with QoS 0 or QoS 1 because it affects the QoS the MQTT
message that will delivered to the MQTT subscriber.

The performance impact of always setting the durable annotation is
negligible.
ansd added a commit that referenced this pull request Apr 22, 2024
This is a follow up to #11012

 ## What?
For incoming MQTT messages, always set the `durable` message container
annotation.

 ## Why?
Even though defaulting to `durable=true` when no durable annotation is
set, as prior to this commit, is good enough, explicitly setting the
durable annotation makes the code a bit more future proof and
maintainable going forward in 4.0 where we will rely more on the durable
annotation because AMQP 1.0 message headers will be omitted in classic
and quorum queues (see #10964)

For MQTT messages, it's important to know whether the message was
published with QoS 0 or QoS 1 because it affects the QoS for the MQTT
message that will delivered to the MQTT subscriber.

The performance impact of always setting the durable annotation is
negligible.
ansd added a commit that referenced this pull request Apr 22, 2024
This is a follow up to #11012

 ## What?
For incoming MQTT messages, always set the `durable` message container
annotation.

 ## Why?
Even though defaulting to `durable=true` when no durable annotation is
set, as prior to this commit, is good enough, explicitly setting the
durable annotation makes the code a bit more future proof and
maintainable going forward in 4.0 where we will rely more on the durable
annotation because AMQP 1.0 message headers will be omitted in classic
and quorum queues (see #10964)

For MQTT messages, it's important to know whether the message was
published with QoS 0 or QoS 1 because it affects the QoS for the MQTT
message that will delivered to the MQTT subscriber.

The performance impact of always setting the durable annotation is
negligible.
mergify bot pushed a commit that referenced this pull request Apr 22, 2024
This is a follow up to #11012

 ## What?
For incoming MQTT messages, always set the `durable` message container
annotation.

 ## Why?
Even though defaulting to `durable=true` when no durable annotation is
set, as prior to this commit, is good enough, explicitly setting the
durable annotation makes the code a bit more future proof and
maintainable going forward in 4.0 where we will rely more on the durable
annotation because AMQP 1.0 message headers will be omitted in classic
and quorum queues (see #10964)

For MQTT messages, it's important to know whether the message was
published with QoS 0 or QoS 1 because it affects the QoS for the MQTT
message that will delivered to the MQTT subscriber.

The performance impact of always setting the durable annotation is
negligible.

(cherry picked from commit e576dd7)
ansd added a commit that referenced this pull request Apr 22, 2024
This is a follow up to #11012

 ## What?
For incoming MQTT messages, always set the `durable` message container
annotation.

 ## Why?
Even though defaulting to `durable=true` when no durable annotation is
set, as prior to this commit, is good enough, explicitly setting the
durable annotation makes the code a bit more future proof and
maintainable going forward in 4.0 where we will rely more on the durable
annotation because AMQP 1.0 message headers will be omitted in classic
and quorum queues (see #10964)

For MQTT messages, it's important to know whether the message was
published with QoS 0 or QoS 1 because it affects the QoS for the MQTT
message that will delivered to the MQTT subscriber.

The performance impact of always setting the durable annotation is
negligible.

(cherry picked from commit e576dd7)
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

Successfully merging this pull request may close these issues.

3 participants