Nservicebus: Native support for FLR counter

[tmasternak]

It looks that for some transports e.g. ASB it might be possible to get delivery count directly from the transport. It could enable better FLR implementation: no data would have to be stored in memory, value would be consistent between scaled-out endpoint instances in competing consumer setup.

One of the possible solutions to enable transport to provide this information would be to enable transport seam to provide custom implementation of FLRStatusStorage.

Some investigation is required though to make sure we can make it work correctly with SLR and Errors.

related to #3032
/cc: @Particular/azure-maintainers

[SeanFeldman]

Clarification: transport would _not_ store anything, but be used as a Reader to get the count.

[timbussmann]

is that mechanism for azure able to yield the same results as the current storage? We only increment on retries by the FLR behavior. What about other re-deliveries, would they count toward a FLR?

[tmasternak]

@timbussmann What do you mean by re-delivery?

[timbussmann]

@tmasternak e.g.context.HandleCurrentMessageLater()

[tmasternak]

@SeanFeldman I think that native support for FLR counter can be a bit more tricky than I thought. What about situation when pipeline throws and we reach FLR? Right now we need to return message to the input queue and move it to the slr/errors when seen next time. It looks that if n='max delivery count'. We would be able to do proper FLR only n-2 times (we loose one try due the fact that we need to move it back to queue before slr/errors kick in).

[timbussmann]

ignore my comment related to context.HandleCurrentMessageLater() since it does no rollback but sends a new message.

[DavidBoike]

I dealt with some of this with SQS and I'm guessing there are some parallels. The server probably stores how many times the message has been peeked at for any reason. This includes any sort of GUI tools used to have a look at the queue. So be aware that the count can probably be incremented for _many_ reasons other than an actual processing attempt.

[timbussmann]

I'm not sure that a native FLR counter support really adds enough value. It seems to solve a very "small" problem for only one or two transports maybe.

[SeanFeldman]

@timbussmann not for all transports, but it adds value to our customers that are on transports that can do it. Not to mention that the in-memory count that is incorrect would be replaced by a proper competing-consumer counter.

[timbussmann]

@SeanFeldman what's the value added by using a different (native) mechanism?

[SeanFeldman]

@timbussmann here

[SeanFeldman]

The server probably stores how many times the message has been peeked at for any reason. This includes any sort of GUI tools used to have a look at the queue. So be aware that the count can probably be incremented for many reasons other than an actual processing attempt.

Disagree @DavidBoike. Tools distinguish between peek and receive. When peek API is used, ASB doesn't increase delivery count, only when receive is used.

[yvesgoeleven]

@tim when instances are scaled out, they can deadletter instead of going to error queue and users have no information why

[SeanFeldman]

There was no update for a while. Can we get back to this issue? Thank you.

[timbussmann]

@tim when instances are scaled out, they can deadletter instead of going to error queue and users have no information why

sorry I can't see where this relates to the FLR retry counter. Can you elaborate?

@SeanFeldman I'm still not convinced that the added effort and complexity is worth it.

[yvesgoeleven]

@timbussmann it fixes a bug, when scaled out (competing consumer). A copy of the FLR counter is maintained in the memory of each instance. causing MaxRetryCount * instancecount as max retry count as instances pick the message up alternatingly. If underlying transport has a DLQ mechanism of it's own, configured to less then MaxRetryCount * instance count, it will DLQ the message before we send it to SLR or error queue.

[tmasternak]

Can we try identifying the scenarios in which we can take advantage of native FLR?. Here is my list:

• User wants FLR + Errors - We can use native mechanisms here
• User wants FLR + SLR + Errors - this is tricky right? SLR should kick in only after number of FLR reached max - not sure if we have access to this info on Azure /cc: @yvesgoeleven @SeanFeldman
• User wants SLR + Errors - there is no need to use native approach.

Is this list valid? Do you think it would make sense to have option: "turn off generic recoverability in core and use native one?". Do you think it we should make sure that core and Azure settings do not contradict each other? What happens if Queue FLR is 2 and user sets 5 in configuration?

[timbussmann]

it fixes a bug, when scaled out (competing consumer). A copy of the FLR counter is maintained in the memory of each instance. causing MaxRetryCount * instancecount as max retry count as instances pick the message up alternatingly.

This is a known behavior which is the same on all transports. I wouldn't consider it a bug but a design decision (it's basically a cache invalidation problem) and I prefer having the same behavior on each transport rather than every transport behaving differently. Same from a documentation perspective.

[SeanFeldman]

@tmasternak

User wants FLR + SLR + Errors - this is tricky right? SLR should kick in only after number of FLR reached max - not sure if we have access to this info on Azure

Both ServiceBus and Storage Queues can provide number of deliveries that took place. SLR is NSB`s concept. So technically max delivery count would be equal to total number of attempts to deliver.

@timbussmann

This is a known behavior which is the same on all transports.

Not quite. With some transports if you dequeue multiple times, it is not going to remove the message from the queue. With ASB if you surpass DeliveryCount, the message is moved to the DLQ as broker assumes that exceeding the set threshold means message is bad.

[tmasternak]

@SeanFeldman that's good news. One additional thing that I realized is that both SLR or Errors work in such a way that they first return message to the queue and will fire on the next receive (this is done to make sure we do not commit receive transaction scope). This causes a small problem (I think) because if we want to have SLR + native FLR the message needs to be delivered native_FLR_max_retries+1. But as I said we should be able to make it work.

[SeanFeldman]

@tmasternak for ASQ not an issue, it has no DLQ (as of today :grinning:). For ASB set delivery count to whatever the "FLR x SLR" value is.

[tmasternak]

@SeanFeldman :+1:

[timbussmann]

With ASB if you surpass DeliveryCount, the message is moved to the DLQ as broker assumes that exceeding the set threshold means message is bad.

that sounds like it's a different behavior than expected? If retries are exceeded, the message should be moved to the errorqueue?

Not quite. With some transports if you dequeue multiple times, it is not going to remove the message from the queue.

sorry, I can't follow. Can you elaborate?

[SeanFeldman]

that sounds like it's a different behavior than expected? If retries are exceeded, the message should be moved to the errorqueue?

It depends. When you have a competing consumer and your custom code has an in-memory counter per consumer, that might not happen. Exactly what @yvesgoeleven was trying to say in his https://github.com/Particular/NServiceBus/issues/3354#issuecomment-190108001

The 2nd item, @timbussmann sorry, poor way to put it. What will happen is the message is moved to a DLQ associated with the processing queue. I.e. there's a DLQ per each queue.

I find it difficult to have this discussion w/o actually having a face to face conversation. Perhaps we should get on a call?

[timbussmann]

@SeanFeldman agreed, can you schedule a meeting?

[danielmarbach]

In my humble opinion we should not spend time on this for this release. If
you really feel this has to be done, flesh out an issue and description and
ping the platform dev squad for a request to prioritize.

[SeanFeldman]

Thanks @danielmarbach
Agreed. Let's park this until core v6 is out.

[ramonsmits]

Seems that this user got bitten by this behavior: http://stackoverflow.com/questions/36303008/nservicebus-messageforwardingincaseoffaultconfig-not-working-as-expected%3E also mentioned this in https://github.com/Particular/NServiceBus.AzureServiceBus/issues/82

[SeanFeldman]

Another case where non-native FLR makes troubleshooting difficult: SO question. This time the reason for a failure is not capture on a DLQed.

On a positive side, in core V6 INFO and WARN messages for FLR will have message Id to correlate with. In core v5 correlation was not possible due to message Id not being captured.
(INFO NServiceBus.Unicast.Transport.TransportReceiver Failed to process message).

[noctovis]

Without Sean's help it may have taken me a whole while longer to figure out what was wrong. So in my case (the SO question posted above), it would have been easier to figure out with an FLR-counter. +1 for this feature!

[timbussmann]

I wonder whether the transport could just provide a custom FLR behavior and disable the default behavior provided by the core?

[SeanFeldman]

@tmasternak I'm all ears :smiley:

[tmasternak]

@SeanFeldman currently it should be possible to substitute the FLR behavior with custom one. There would be however a not ideal relation between MaxDeliveryCount and MaxFLRRetries. We would have MaxFLRRetries = MaxDeliveryCount-2 but could prevent the problem of messages being silently moved to DLQ.

[SeanFeldman]

I've noticed that core v6 is logging each message processing attempt with message ID. At least this will allow to correlate between DLQed message and log files.
Saying that, I'd love to sync to understand the implication on FLR. When your time permits.

[SeanFeldman]

With the new Retries seam implemeted, can this issue be closed?

[andreasohlund]

@Particular/azure-service-bus-maintainers @Particular/azure-storage-queues-maintainers should we raise issues in your repos and connect them to the upcoming release since this change in essence just makes it possible to use native FLR but your transports are the one that actually use it?

[yvesgoeleven]

@andreasohlund we have this issue (and closed it already), https://github.com/Particular/NServiceBus.AzureServiceBus/issues/11

[Scooletz]

That was addressed in ASQ when aligning the transport to the latest shape of the recoverability seam and is present in the recent release branch https://github.com/Particular/NServiceBus.AzureStorageQueues/blob/release-7.0.0/src/Transport/ReceiveStrategy.cs#L28 @andreasohlund

[andreasohlund]

@Scooletz that's fine, but how would asq user know? (should we include it in ASQ release notes?)

[Scooletz]

@andreasohlund IMO it's an internal usage of the DequeueCount for the recoverability purposes, hence no need of informing an asq user. If you find it necessary I may create a specific issue for it, pointing at this line.

[andreasohlund]

@Scooletz but doesn't this change mean that ASQ users now get more accurate "Immediate" retries when scaled out?

current version: number of retries <= number of instance * num configured retries

upcoming version; number of retries == num configured retries

[Scooletz]

That's true @andreasohlund . Now in scale out scenarios, you'll get an accurate number of "Immediate".

[andreasohlund]

So raise an ASQ issue and attach it to the 7.0.0 milestone?

[Scooletz]

@andreasohlund The issue has been created and marked as done. It's been attached to the milestone 7.0.0 https://github.com/Particular/NServiceBus.AzureStorageQueues/issues/145

[tmasternak]

current version: number of retries <= number of instance * num configured retries
upcoming version; number of retries == num configured retries

@andreasohlund I think it actually is:
current version: number of retries ∈ [0, inf]
new version: number of retries ∈ [0, num configured retries]