[ 
https://issues.apache.org/jira/browse/QPIDJMS-608?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17952254#comment-17952254
 ] 

Timothy A. Bish edited comment on QPIDJMS-608 at 5/16/25 5:06 PM:
------------------------------------------------------------------

{noformat}
<type name="header" class="composite" source="list" provides="section">
    <descriptor name="amqp:header:list" code="0x00000000:0x00000070"/>
    <field name="durable" type="boolean" default="false"/>
    <field name="priority" type="ubyte" default="4"/>
    <field name="ttl" type="milliseconds"/>
    <field name="first-acquirer" type="boolean" default="false"/>
    <field name="delivery-count" type="uint" default="0"/>
</type>{noformat}
If the client sends the Header when it should and fills in the durable value 
when it needs to in order to meet the specification defined behavior then I 
don't see that there is an improvement to be made here. 


was (Author: tabish121):
{noformat}
<type name="header" class="composite" source="list" provides="section">
    <descriptor name="amqp:header:list" code="0x00000000:0x00000070"/>
    <field name="durable" type="boolean" default="false"/>
    <field name="priority" type="ubyte" default="4"/>
    <field name="ttl" type="milliseconds"/>
    <field name="first-acquirer" type="boolean" default="false"/>
    <field name="delivery-count" type="uint" default="0"/>
</type>{noformat}
If the client send the Header when it should and fills in the durable value 
when it needs to in order to meet the specification defined behavior then I 
don't see that there is an improvement to be made here. 

> 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

Reply via email to