Particular / NServiceBus.AmazonSQS

An AWS SQS transport for NServiceBus.
Other
35 stars 20 forks source link

Shouldn't the message pump TTBR check be done by the core in the pipeline? #153

Closed ramonsmits closed 7 years ago

ramonsmits commented 7 years ago

The message pump checks if the TTBR is passed.

https://github.com/Particular/NServiceBus.AmazonSQS/blob/d205625ba4cd1183b52cadf454c7788117c897d1/src/NServiceBus.AmazonSQS/MessagePump.cs#L161-L179

Isn't this a concern that should be performed by core thus be one of the first steps in the pipeline?

I have some drain logic in my tests, the idea is that I drain the queue by starting the endpoint and waiting for messages. If no messages are processed then I assume the queue is empty but in reality, it isn't and a lot of messages were drained by the message pump without any kind of notification except a log entry. Because the pipeline isn't hit I cannot detect this and shut down the endpoint instance then assuming the queue is empty and start my test but then the queue still isn't empty......

public class ShortcutBehavior : Behavior<ITransportReceiveContext>
{
    public static bool Shortcut;
    public static long Count;

    public override Task Invoke(ITransportReceiveContext context, Func<Task> next)
    {
        if (Shortcut)
        {
            Interlocked.Increment(ref Count);
            return Task.FromResult(0);
        }
        return next();
    }
}

For now my only solution is not to set a TTBR on the message to circumvent the message pump TTBR check.

danielmarbach commented 7 years ago

I don't think this is a core concern. The transport is responsible to translate TTBR concept to the native thing or like done here in the pump to discard messages that have it set. For you tests wouldn't it be safer to recreate the queues? You could use different queue prefixes for each run to avoid having to wait the 60 seconds until you can recreate the queues, see https://docs.particular.net/transports/sqs/configuration-options?version=sqs_4#queuenameprefix

danielmarbach commented 7 years ago

@ramonsmits if you have no objections I would like to close this one.

ramonsmits commented 7 years ago

@danielmarbach thanks for that explanation. I now think that does make sense.