pardahlman / RawRabbit

A modern .NET framework for communication over RabbitMq
MIT License
747 stars 144 forks source link

NoAck not working as expected? #243

Closed msnelling closed 7 years ago

msnelling commented 7 years ago

Hi, I am using a subscriber with the following configuration

                cfg.WithQueue(
                        q => q
                            .WithName("tmq")
                            .WithDurability()
                    )
                    .WithExchange(
                        e => e
                            .WithName("tmx")
                            .WithType(ExchangeType.Fanout)
                            .WithDurability()
                    )
                    .WithSubscriberId("")
                    .WithRoutingKey("")
                    .WithNoAck());

Posting a message to the queue using the RabbitMQ management website and it is throwing an exception on deserialization (this is expected in my case).

I'm using custom error handling to avoid faulty messages being published to the error exchange. I would simply like them to remain on the queue unacknowledged.

However, what I'm seeing is the message being Ack'd anyway and disappearing off the queue.

This is the OnSubscriberExceptionAsync function code in the custom error strategy.

        public override async Task OnSubscriberExceptionAsync(
            IRawConsumer consumer, 
            SubscriptionConfiguration config,
            BasicDeliverEventArgs args, 
            Exception exception)
        {
            if (!config.NoAck)
            {
                consumer.Model.BasicAck(args.DeliveryTag, false);
                consumer.AcknowledgedTags.Add(args.DeliveryTag);
            }

            Log.LogError("Exception thrown in subscriber: ", exception);
            await Task.FromResult(0);
        }
    }

In the debugger I can see that my custom strategy isn't performing the Ack.

pardahlman commented 7 years ago

Hello @msnelling - thanks for reporting this!

I believe that you might have misunderstood how NoAck works. It is a feature that, when active, tells the broker/server that no acknowledgement will be sent, but rather the message is automatically ack'ed. The property is poorly named, which is why it is renamed to AutoAck in an upcoming version of RabbitMQ.Client (see https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/297).

Hope this helps!

pardahlman commented 7 years ago

On a different note: when you say "I would simply like them to remain on the queue unacknowledged" do you mean that you want the client in which the exception occurred to keep the message unack'ed? If that's the case, then I would say it is a bit unconventional approach, as the message will be "processed" by the client forever. The only way to get the client to "release" the message would be to close the consumer's channel (by closing the channel, connection or simply restart the service).

Perhaps you've already seen this, but there is a requeue with delay feature in RawRabbit. Not sure if it is close to what you want to do.

msnelling commented 7 years ago

Yes I can see how that sounds, the reason for not immediately Ack'ing the message is so that if for some reason the client crashes, or was running an older version of our protocol by mistake, the message isn't lost and is waiting in the queue for when the client restarts. We do Ack the message, but only after we have successfully processed it. Ordering of our messages is important too, we wouldn't want a message to be requeued and then be processed out of order.

pardahlman commented 7 years ago

Perfect! Well, I think you will get the desired behavior by removing the NoAck from the subscriber configuration and in your custom error strategy simply do this

            if (!config.NoAck)
            {
                // don't ack here, but put in in the list of acknowledged tags;
                consumer.AcknowledgedTags.Add(args.DeliveryTag);
            }
msnelling commented 7 years ago

Thanks!