[
https://issues.apache.org/jira/browse/QPIDJMS-608?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
David Ansari closed QPIDJMS-608.
--------------------------------
Fix Version/s: (was: 2.8.0)
Resolution: Won't Do
That's fair. I also changed my mind slightly: I think it's acceptable for the
AMQP spec to define durable to be false by default for maximum performance to
save a few bytes on the wire and a bit of (un)marshalling on client and server.
Apps won't directly encode AMQP messages, they use libraries, and therefore the
right place to be safe by default is in the libraries. Hence,
[https://github.com/rabbitmq/rabbitmq-server/pull/13918] will change RabbitMQ
to assume messages being non-durable if the header section is omitted pushing
the "safe by default" behaviour from the broker back to the client libs. JMS is
already safe by default because "The message producer's default delivery mode
is PERSISTENT."
Thank you for this discussion and your prompt replies.
We can now close this ticket.
> Send AMQP header section with durable field
> -------------------------------------------
>
> Key: QPIDJMS-608
> URL: https://issues.apache.org/jira/browse/QPIDJMS-608
> Project: Qpid JMS
> Issue Type: Improvement
> Components: qpid-jms-client
> Affects Versions: 2.7.0
> Reporter: David Ansari
> Priority: Major
>
> I would like to suggest the following improvement.
> h1. What?
> The Qpid JMS client should always send the AMQP 1.0 header section with at
> least the durable field explicitly set to true or false.
> h1. Why?
> The current behaviour is that the header section is completely omitted from
> being sent (transmitted across the wire), if the JMS app opted in to send the
> message as NON_PERSISTENT. This complies with the AMQP 1.0 spec and is
> functionally correct because all the AMQP 1.0 header section field defaults
> will apply at the receiver.
> However, the AMQP 1.0 spec also states the following:
> "The header section carries standard delivery details about the transfer of a
> message through the AMQP network. If the header section is omitted the
> receiver MUST assume the appropriate default values (or the meaning implied
> by no value being set) for the fields within the header {*}unless other
> target or node specific defaults have otherwise been set{*}."
> This means that AMQP 1.0 brokers could set other defaults.
> RabbitMQ interprets {{durable}} as {{true}} by default to be safe by default.
> That's because beginners could easily forget setting the {{durable}} field to
> {{true}} resulting in unexpected message loss.
> In my personal opinion, the fact that the AMQP 1.0 spec defines the default
> for durable as false was a mistake:
> <field name="durable" type="boolean" default="false"/>
> In my opinion, JMS got it right by using a default of PERSISTENT for the
> message producer.
> Anyway, I suggest that this JMS client should always explicitly send the
> {{durable}} field to allow RabbitMQ to store the message non-durably. In
> addition, message selectors can filter on JMSDeliveryMode. Therefore,
> RabbitMQ needs to know whether durable is true or false.
> In other words, do not omit this field when marshalling.
> The performance penalty of sending the durable field is negligible.
> h1. How?
> I tested that the following patch works as expected:
> {code:java}
> diff --git
> a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpHeader.java
>
> b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpHeader.java
> index 622b5e4c..7deab521 100644
> ---
> a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpHeader.java
> +++
> b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpHeader.java
> @@ -75,26 +75,17 @@ public final class AmqpHeader {
> } public Header getHeader() {
> - Header result = null;
> -
> - if (!isDefault()) {
> - result = new Header();
> - // As we are now definitely sending a Header, always
> - // populate the durable field explicitly rather than
> - // potentially default it if false.
> - if(Boolean.TRUE.equals(durable)) {
> - result.setDurable(Boolean.TRUE);
> - }
> - else {
> - result.setDurable(Boolean.FALSE);
> - }
> -
> - result.setPriority(priority);
> - result.setFirstAcquirer(firstAcquirer);
> - result.setTtl(timeToLive);
> - result.setDeliveryCount(deliveryCount);
> + Header result = new Header();
> + if(Boolean.TRUE.equals(durable)) {
> + result.setDurable(Boolean.TRUE);
> }
> -
> + else {
> + result.setDurable(Boolean.FALSE);
> + }
> + result.setPriority(priority);
> + result.setFirstAcquirer(firstAcquirer);
> + result.setTtl(timeToLive);
> + result.setDeliveryCount(deliveryCount);
> return result;
> } {code}
>
> h1. Additional Context
> The same change was done in the AMQP 1.0 Go client:
> [https://github.com/Azure/go-amqp/issues/330]
>
> Please let us know what you think. We are happy to contribute a PR with tests
> directly to [https://github.com/apache/qpid-jms]
> Thank you for your collaboration!
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
