Nservicebus: SatelliteReceiver required transport transaction mode isn't validated against supported modes

[bording]

Plan of Attack

• Obsolete with warning AddSatelliteReceiver which allows passing in a transaction mode
• Add a check inside the obsoleted AddSatelliteReceiver which throws if the requested transaction mode is different from the main input one
• Add an API AddSatelliteReceiver which doesn't allow to specify the transaction mode; the core will automatically set the main input queue transaction mode

Original content

FeatureConfigurationContext.AddSatelliteReceiver lets you pass in an a requiredTransportTransactionMode, but when the TransportReceiver for the satellite is created in StartableEndpoint.CreateReceivers, the required mode isn't validated against the supported transaction modes of the transport.

This means the transport can get a request to run in a transaction mode that it can't support.

[bording]

Seems like this should be fixed along with #4213 in 6.1.0.

[bording]

Thinking about this more, I'm questioning why we're letting satellites specify a transaction mode that is different than the user-specified transaction mode in the first place. Why should a feature that declares a satellite receiver get to decide this?

For example, I know ASB has to to do a lot of work up front to figure out what transaction mode it can use for the main receiver, and suddenly getting a different value from what the user has requested is going to be a problem.

//cc @SeanFeldman

[andreasohlund]

Not sure I agree. A satellite is in complete control of what consistency is needed for the operation(s) its performing. If the operation performed doesn't need transactions (idempotent) why would we force the transactions to be used when not needed just because the endpoint needs them?

On 15 Oct 2016, at 00:45, Brandon Ording [email protected] wrote:

Thinking about this more, I'm questioning why we're letting satellites specify a transaction mode that is different than the user-specified transaction mode in the first place. Why should a feature that declares a satellite receiver get to decide this?

For example, I know ASB has to to do a lot of work up front to figure out what transaction mode it can use for the main receiver, and suddenly getting a different value from what the user has requested is going to be a problem.

//cc @SeanFeldman

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[SeanFeldman]

@andreasohlund how about the other way around, the endpoint is configured to use a lower transaction mode that satellite is requesting? If an endpoint is running in Receive only mode, and satellite is requesting Atomic mode, it won't work since transport will be adjusted to the endpoint`s configuration.

[bording]

@andreasohlund The key assumption there is that a transport can support more than one transaction mode simultaneously. This appears to be an assumption that we don't have validated in a test anywhere, either.

As @SeanFeldman is saying, that assumption isn't true for all transports currently!

[andreasohlund]

:+1: to document this using tests.

I'd say a transport should throw at start up when requested to run in a mode or combination of modes that are not supported?

On 15 Oct 2016, at 17:35, Brandon Ording [email protected] wrote:

@andreasohlund The key assumption there is that a transport can support more than one transaction mode simultaneously. This appears to be an assumption that we don't have validated in a test anywhere, either.

As @SeanFeldman is saying, that assumption isn't true for all transports currently!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[SeanFeldman]

Documentation - yes.
Call to explain - yes.
Having a discussion/ conversation - yes.

[danielmarbach]

I think from a core perspective we haven't put enough thought into how satellites should behave and how autonomous they can be in terms of choosing their own transaction mode. How about I organize a meeting for next week and invite the core maintainers as well as the transport maintainers into a call where we agree on a future direction / common behavior and raise follow up issues after that call? // cc @Particular/azure-service-bus-maintainers @Particular/azure-storage-queues-maintainers @Particular/sqlserver-transport-maintainers @Particular/rabbitmq-transport-maintainers

[bording]

@danielmarbach Do you still want to try and organize a call for this?

[danielmarbach]

I saw no response until now when I found out that transport maintainers gave a thumbs up. I'll see what I can do

[danielmarbach]

We had a call with @bording @timbussmann @SeanFeldman @boblangley this is the outcome and action points:

We concluded that if only it would make sense for a satellite to lower the transaction mode for performance reasons. We think that is not a use case we want to support because the future of satellites is currently not well known. For example, we could envision that you could just build a message pump and a dispatcher yourself by accessing the transport infrastructure. In the light of that we will go ahead with the following:

• Obsolete with warning AddSatellite which allows passing in a transaction mode
• Add a check inside the obsoleted AddSatellite which throws if the requested transaction mode is different from the main input one
• Add an API AddSatellite which doesn't allow to specify the transaction mode; the core will automatically set the main input queue transaction mode

Meeting recording

I'll update the description of this issue.

[danielmarbach]

@andreasohlund see https://github.com/Particular/NServiceBus/issues/4221#issuecomment-258182646

[andreasohlund]

I'm good with the proposed actions

[SzymonPobiega]

@danielmarbach I [this comment] I summarized the problems one would encounter when trying to put together a pump and a dispatcher. I'll prepare a pull that addresses them.