Nservicebus: Should a value specified in the TransportInfrastructure.TransactionMode imply that all lower modes are also supported?

[indualagarsamy]

Based on a discussion with @bording, @yvesgoeleven, @timbussmann on Slack and further discussion with @bording, here are details:

Currently, the core defines the following as transport transaction modes
https://github.com/Particular/NServiceBus/blob/develop/src/NServiceBus.Core/Transports/TransportTransactionMode.cs

• None (0)
• ReceiveOnly (1)
• SendsAtomicWithReceive (2)
• TransactionScope (3)
• User specifies TransportTransactionMode in the EndpointConfiguration when supplying the transport.
• This TransportTransactionMode is passed on by the core to the IPushMessages implementation of the transport (in the Init method)
• The PushSettings.RequiredTransactionMode is set to the value that the user passed in.

An exception will be thrown if the user configures a transaction mode that is not supported by the transport.
For example, If “TransactionScope” is set as the transaction mode when using RabbitMQ transport, the core will throw an exception.

The problem:
The core assumes that when a transport says it supports a given mode, it supports all lesser modes.
Example: If a transport says it supports, TransactionScope which is enum 3, the core assumes that 0, 1 and 2 are also supported by the transport.

https://github.com/Particular/NServiceBus/blob/5d8c6c03b6ae8815a800d1c560440e82004a484b/src/NServiceBus.Core/Transports/TransactionModeSettingsExtensions.cs#L27

That assumption would be wrong in certain transports. See related issue:
https://github.com/Particular/NServiceBus.AzureServiceBus/issues/101

Questions:

1. Is this assumption by the core even a valid one? Is this assumption being made on past behavior from MSMQ?

Proposed:
The transport should specify the modes that it supports. If the user-specified option isn’t on the supported list from the transport, then the core should throw a NotSupported exception.

The transport shouldn’t have to make any assumptions about trying to match a lower transaction mode than what’s on the supported list.

[indualagarsamy]

@yvesgoeleven - Please make sure that I've captured everything we discussed if not, feel free to update the issue.

@Particular/nservicebus-maintainers @Particular/transport-maintainers - please chime in. I marked this for CoreV6 because it could affect how we are doing things.

[andreasohlund]

Yes the assumption was that the transport provides the "highest" supported mode and that all lower modes are supported. User can then request a mode <= max(SupportedByTransport)

Think we're good regarding SqlServer and Rabbit. I tried to look at https://github.com/Particular/NServiceBus.AzureServiceBus/issues/101 but couldn't figure out which combinations that wasn't supported?

@Particular/azure-maintainers can you give some examples where the order theory is violated?

[yvesgoeleven]

@indualagarsamy small nuance for ASB, it's not that we can't but at the time that the transport receives the required transaction mode, it isn't able to change it's settings anymore in order to lower the perceived transactional behavior as the settings are locked at that time. If we had this info earlier, we would still be able to do it.

[andreasohlund]

Should we pass it to https://github.com/Particular/NServiceBus/blob/develop/src/NServiceBus.Core/Transports/TransportInfrastructure.cs#L35 ?

[yvesgoeleven]

@andreasohlund as you know there is no such thing as a transaction in azure, there is just a bunch of settings that make asb behave in a way that looks like one. If the user requires a different (lower) level then how asb transport is configured, we should lower it by overriding some of those settings, but we can't anymore as the settingsholder is locked at the point where we get the required moden (alternatively we cloud throw telling the user to lower it themselves)

[yvesgoeleven]

@andreasohlund ideally in initialize as that is the last stage where we can change settings (is it in settings already perhaps, can I pull it out of there?)

[andreasohlund]

Its already in settings: https://github.com/Particular/NServiceBus/blob/develop/src/NServiceBus.Core/Unicast/Transport/Config/TransportExtensions.cs#L110

[yvesgoeleven]

ok, if that is the same as RequiredTransactionMode (and not our SupportedTransactionMode), then I'll pull it out and adjust accordingly

[bording]

Think we're good regarding SqlServer and Rabbit

Based on not being able to use the auto-acknowledgement "feature" of RabbitMQ for reasons, it's actually somewhat tricky to support None in the Rabbit transport. The manual acknowledgements also tie into the server -> client flow control mechanism, so adjusting when an ack is sent can have a larger impact.

If a transport could specify the exact list of transaction modes supported, I'd actually want to have RabbitMQ support only ReceiveOnly, because there's always going to be a chance for a message to be requeued, which I understand shouldn't happen if the transaction mode is None.

[yvesgoeleven]

None is plain dangerous, I wouldn't mind being able to exclude that one as well.

[indualagarsamy]

I think a list of supported options by the transport and the core checking that is much better than a > check based on a enum value. I think the assumption that the core is making is incorrect.

[andreasohlund]

None is plain dangerous, I wouldn't mind being able to exclude that one as well.

One use case for None that I've seen is to avoid the receive transaction timing out. What's the max peek look you can do on ASB/ASQ?

@bording is there some timeout in rabbit where unacked messages are redelivered?

[bording]

is there some timeout in rabbit where unacked messages are redelivered?

AFAIK there isn't a timeout. It waits until the channel is closed, and any unacked messages at that point are then re-queued.

[indualagarsamy]

One use case for None that I've seen is to avoid the receive transaction timing out. What's the max peek look you can do on ASB/ASQ?

@andreasohlund - I think from a code perspective, if the transaction times out, the user explicitly setting a larger timeout is a much better to understand than trying to figure out what None means.

[yvesgoeleven]

@andreasohlund 30 seconds by default, 5 mins max

[andreasohlund]

I think from a code perspective, if the transaction times out, the user explicitly setting a larger timeout is a much better to understand than trying to figure out what None means

Not sure that's always possible/feasible? (mucking around with maxtransactions and stuff in machine.config, also not sure if how much you can dial up eg. msmq native transactions)
Also what about users wanting "at most once" ?
What about performance and resource usage? (no tx receives is more performant and resource effective on most transports, @yvesgoeleven not familiar with ASB/ASQ how much do you save there?)

[SeanFeldman]

@andreasohlund ASQ: up to 7 days

[yvesgoeleven]

@andreasohlund there is no such thing as a transaction in ASB/ASQ, so it makes no difference

[andreasohlund]

Ok so you always have to peaklock + receive? There is no plain receive in a single roundtrip?

[SeanFeldman]

@andreasohlund there's a peak-lock and receive. But it's not about resources and cost.

[yvesgoeleven]

there is receive, lock, process, delete (commonly named peek-lock), and there is receive, delete, process

[yvesgoeleven]

lock means message turns invisible

[andreasohlund]

So you would never use the "ReceiveAndDelete" mode? https://msdn.microsoft.com/library/azure/microsoft.servicebus.messaging.receivemode.aspx

[yvesgoeleven]

I wouldn't no, I see no reason why to put yourself at risk losing the message

[andreasohlund]

But that's kind of what TransactionMode.None means? "Just give me the message as efficient as possible, I can afford to loose it so just give it your best shot"

[andreasohlund]

Would you save money using ReceiveAndDelete? (assuming its one less storage op)

[yvesgoeleven]

A tiny amount on storage, for ASB it doesn't matter there you pay per message now

[andreasohlund]

What about latency? (saving one round trip) Any perf numbers comparing receives with ReceiveAndDelete vs PeekLock?)

[andreasohlund]

Not against allowing transports to not support it but I definitely see use cases for most transports we have so I don't see us dropping it as a possible option since it makes a lot of sense on MSMQ and SQL

[yvesgoeleven]

it would save a roundtrip yes, so it would shave off 50+ ms for each message

[andreasohlund]

it would save a roundtrip yes, so it would shave off 50+ ms for each message

So why shouldn't we do this if the users ask for None?

[yvesgoeleven]

We can support it, we did in the past, I just consider it a dangerous thing to do...

[andreasohlund]

Well, its dangerous on all transports but there are valid use cases

[andreasohlund]

@bording any perf numbers on rabbit comparing None vs ReceiveOnly? (Does AutoACK save one roundtrip there as well?)

[bording]

any perf numbers on rabbit comparing None vs ReceiveOnly? (Does AutoACK save one roundtrip there as well?)

Auto-ack mode would save on not needing to send back an ack message to the server, but auto-ack mode is basically not a usable option at all. If auto-ack is turned on, we have no way to prevent the server from blasting us with messages, potentially causing an OOM exception if the incoming rate is too fast for us to process them.

I'm also currently relying on the manual-ack flow control (BasicQos) as part of my maxConcurrency rate limiting.

In the current v6 version on develop, I don't even have a way to enable auto-ack. From the transport perspective, None and ReceiveOnly act identically.

[indualagarsamy]

@andreasohlund - I think it makes sense to have a call with the @Particular/transport-maintainers and especially with @yvesgoeleven and @bording about this.

My concern is this.
For some transports, i.e. RabbitMQ and Azure, None makes no sense
For some transports, i.e. MSMQ and SQL, it makes sense.
But we're enforcing this at a core level?

Circling back to the original question:

The transport shouldn’t have to make any assumptions about trying to match a lower transaction mode than what’s on the supported list.

Can we have a more focused call?

[andreasohlund]

RabbitMQ and Azure, None makes no sense

Don't agree, like @yvesgoeleven just mentioned above None makes sense to switch to ReceiveAndDelete which ensures "at most once" and also shaves off 50ms per message.

Yes for rabbit we can't use autoack (as it seems) since the server can overload the client with messages. But we are acking them manually before processing the message to give users "at most once" since they won't reappear if process crash or the channel is closed prematurely. Even though this is a roundtrip for the client this still offloads the server since its allowed to remove acked messages so this has a positive effect on resource usage.

Given the above it seems that the order "kind of works", but yes providing the explicit list seem like a more powerfull api. But as it stands all transports would support the same set as today so it doesn't seem urgent?

Do we still need a call? (seems like the the main disagreement is about not supporting AtMostOnce but removing that would be a quite big breaking change since we always supported it so I can't see it making it into v6?)

Perhaps start that discussion async first?

[andreasohlund]

None and ReceiveOnly act identically.

Not true, since we ack the message before processing, the server will be able to remove those messages sooner and use less resources?

[andreasohlund]

@indualagarsamy ok to remove the v6 launch label since the current API seems to be close enough? (and raise a new issue to make it a "list" of supported modes)

[indualagarsamy]

@andreasohlund - We need a call.
Because there's differing points of view which I don't believe we can address in an async way. I believe a call with the right people will clear the confusion and based on that, I am ok with either closing this issue and opening a different one.
The people on the call need to be @bording @yvesgoeleven and you.

[bording]

I know that I at least wouldn't mind a call to sort out what is and isn't possible regarding guarantees around at-most-once delivery in the different versions of the rabbit transport.

[andreasohlund]

I've talked to @yvesgoeleven and we're aligned on None making sense for ASB and ASQ. I'll get on a call with @bording and align on rabbit. I'll put the v6 label back if needed

[bording]

After talking with @andreasohlund, it looks like we've decided that TransportTransactionMode.None doesn't have to mean a 100% strict at-most-once guarantee, so that makes it ok to claim that the RabbitMQ transport supports it.

Instead, we'll try to do whatever performance optimizations at the expense of reliability that are possible.

I still think that an explicit list of supported transaction modes is a better overall api design, but it seems that the need for it to happen now has become less critical.

[indualagarsamy]

Closing this issue based on comments from @bording and @andreasohlund.
I'll open a more specific issue that targets having an explicit list of supported transaction modes for better overall API design.