[
https://issues.apache.org/jira/browse/QPIDJMS-608?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17952550#comment-17952550
]
Robbie Gemmell commented on QPIDJMS-608:
----------------------------------------
If an option is to be added, it really seems like it should be added on the
RabbitMQ/server side. If you choose to have the broker defaulting to doing the
exact opposite of what the spec clearly intends by default, thats , but it
seems like its then the broker that should then be configurable here, not all
the clients. I know of several clients that do this, actually most if not all
I'm familiar with (another of the reasons qpid-jms also does it), and that in
some cases perhaps will have for over a decade. I don't think it makes sense to
alter various clients, and then in turn applications using them, to work around
a deliberate broker-specific divergence with expected spec default behaviour
that many clients used as a result; it really seems like its the broker that
should be adjustable to the specs intended default.
> 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
> Fix For: 2.8.0
>
>
> 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: [email protected]
For additional commands, e-mail: [email protected]
