[ 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: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org