Send field when user sets durable=false

[ansd]

When durable=false, the durable field is omitted in

go-amqp/message.go

Line 331 in 1003610

{Value: &h.Durable, Omit: !h.Durable},

This complies with the spec given that the spec defaults to false for durable.

However, the spec also mentions:

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 brokers could set other defaults.
RabbitMQ 4.0 interprets durable as true by default to be safe by default.
Beginners could easily forget setting the durable field to true resulting in unexpected message loss. This applies to all brokers, not only RabbitMQ.

Hence, I suggest that this client lib should explicitly send the durable field when the user sets this field to false to allow RabbitMQ to store the message non-durable (if explicitly requested by the user).
In other words, do not omit this field when marshalling.

You could use pointers to differentiate between field is set vs field is unset.

The performance penalty of sending this field is negligible.

[jhendrixMSFT]

According to the AMQP 1.0 spec, the absence of a value MUST be interpreted as the default value which would be false in this case. However, this part is of interest.

"unless other target or node specific defaults have otherwise been set"

I believe this means that when creating the link, a different default can be specified. I suspect this ties in with the target's durability setting but am not 100% sure. Are you setting TargetDurability when creating the sender?

[ansd]

Thank you for your reply @jhendrixMSFT.

Are you setting TargetDurability when creating the sender?

I'm not.

I believe this means that when creating the link, a different default can be specified. I suspect this ties in with the target's durability setting but am not 100% sure.

Possibly. The spec is very vague here. The "safest" Terminus Durability setting is unsettled-state which is defined as:

In addition to the existence and configuration of the terminus, the unsettled state for durable messages is retained durably.

Since Terminus Durability is only about the unsettled state, and not about whether the message itself is durable (e.g. written to disk by the target queue), I don't think the spec refers to Terminus Durability.

unless other target or node specific defaults have otherwise been set

I interpret this sentence in the spec as follows: Nodes, for example queues in messaging brokers, can decide themselves what the default value for durable is.

RabbitMQ 4.0 interprets durable as true, by default. RabbitMQ 4.0 is therefore safe by default. This avoid accidental message loss by users / client apps / client libs which happen to forget to explicitly set durable=true on every individual message. Being safe by default is a better practice than being fast by default IMO.

That said, a client app should still be able to overwrite the default by explicitly allowing a message to be non-durable. If the client app using this Go client lib explicitly sets the message as non-durable, I'd wish that this Go lib explicitly sets and marshals the durable=false field sending it to the broker instead of omitting it.

To sum up:

• There is no bug in this library, it complies with the spec.
• However, my feature request is that whenever a user explicitly sets the durable field, that the durable field is always explicitly marshalled and sent to the broker, instead of omitting it (irrespective of the spec's default value).

[jhendrixMSFT]

Since Message.Header is pointer-to-type and nil by default, perhaps what we can do here is when it's not nil to send the values verbatim. In other words, when Message.Header is nil we assume that the peer assumes the default values per spec, and when not nil, you get what you ask for (i.e. pay-for-play).

We do have one rough edge here though. The zero-value for Priority is ambiguous (see #313). Other fields might be impacted too but I haven't dug into them. We could probably work around this by adding a constructor func for MessageHeader that populates the appropriate default values. It's not ideal but would avoid a breaking change by making some/all of the fields pointer-to-type.

CC @richardpark-msft

[ansd]

Thanks.

Since Message.Header is pointer-to-type and nil by default, perhaps what we can do here is when it's not nil to send the values verbatim. In other words, when Message.Header is nil we assume that the peer assumes the default values per spec, and when not nil, you get what you ask for (i.e. pay-for-play).

This sounds great, yes.

by making some/all of the fields pointer-to-type

All header fields are optional according to the AMQP spec.
Ideally, all header fields are pointer-to-type. This way, a nil in Go translates beautifully to the AMQP null value on the wire. All non-nil Go fields can be encoded to non-null AMQP values.
For example, applying this logic to the Priority field means

• if nil in Go, send null in AMQP (broker then interprets it as 4)
• if 0 in Go, send 0 (ubyte) in AMQP
• if 4 in Go, send 4 (ubyte) in AMQP
• ...

(But yeah, it breaks the API unfortunately.)