[ https://issues.apache.org/jira/browse/QPIDJMS-608?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17952231#comment-17952231 ]
Timothy A. Bish commented on QPIDJMS-608: ----------------------------------------- If the JMS sender is configured to send PERSISTENT then the header is included is it not? This follows the specification and the remote should interpret the message it receives according to the specification. Whether or not you agree with the specification it does say what it says and as far as I can tell the library is following it. The encoding and decoding of the header is measurable over time and was handled this way to avoid that cost as well as the GC overhead of creating Header object that simply state what the specification defines as the default. If the RabbitMQ broker needs the header to function then it could create and insert one when decoding an incoming message instead of trying to enforce other libraries to do that work for it. > 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