Masstransit: Batching retry misbehavior

[gschuager]

Retry policy is being incorrectly enforced by batching consumers.

I created a failing unit test here: https://github.com/gschuager/MassTransit/blob/batching-retry/tests/MassTransit.Tests/Batch_Specs.cs#L290

I also tried to fix the issue by commenting some code from BatchConsumerMessageSpecification but then faulting behavior breaks, for which I've added this other test: https://github.com/gschuager/MassTransit/blob/batching-retry/tests/MassTransit.Tests/Batch_Specs.cs#L322

Additionally I've seen the following exception, which I suspect to be related, in my production application:

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
   at System.Collections.Generic.List`1.Enumerator.MoveNext()
   at System.Linq.Enumerable.SelectListIterator`2.MoveNext()
   at System.Threading.Tasks.Task.WhenAll(IEnumerable`1 tasks)
   at MassTransit.Context.RetryConsumeContext.NotifyPendingFaults()
   at MassTransit.Pipeline.Filters.ConsumeContextRetryContext`2.RetryFaulted(Exception exception)
   at GreenPipes.Filters.RetryFilter`1.Attempt(TContext context, RetryContext`1 retryContext, IPipe`1 next)
   at GreenPipes.Filters.RetryFilter`1.GreenPipes.IFilter<TContext>.Send(TContext context, IPipe`1 next)
   at GreenPipes.Filters.TeeFilter`1.<>c__DisplayClass5_0.<<Send>g__SendAsync|1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at GreenPipes.Filters.OutputPipeFilter`2.SendToOutput(IPipe`1 next, TOutput pipeContext)
   at GreenPipes.Filters.OutputPipeFilter`2.SendToOutput(IPipe`1 next, TOutput pipeContext)
   at MassTransit.Pipeline.Filters.DeserializeFilter.Send(ReceiveContext context, IPipe`1 next)
   at GreenPipes.Filters.RescueFilter`2.GreenPipes.IFilter<TContext>.Send(TContext context, IPipe`1 next)

This can be reproduced using: https://github.com/gschuager/MassTransitBatchTest/blob/master/MassTransitTest/Program.cs

[phatboyg]

While being exposed by batching, I don't think this is batching related. I have already worked up a fix, however, that I'm testing for this specific issue (collection modified by subsequent faults generated by other batches that have been grouped).

I hope you realize that the way you are using batching is far beyond the original design, and it's causing a high degree of rework. The fact that you split the message batch into groups, and each group is its own batch and may fault is going to eventually cause all kinds of confusing behavior.

[gschuager]

Thanks in advance, but I'm not sure I follow you... this issue is unrelated to the grouping feature request.

Could this be caused by the retry filter being configured in both the batching and the non-batching pipeline? (not sure if that even makes sense); I get a 10 instead of a 2 here: https://github.com/gschuager/MassTransit/blob/batching-retry/tests/MassTransit.Tests/Batch_Specs.cs#L301

[phatboyg]

Probably, you shouldn't be configuring it for both, imho.

[gschuager]

I'm not doing that explicitly, that's the issue :)

[phatboyg]

Yeah, that's a problem for sure. 10 seems really high though, like way too high.

[phatboyg]

So I'm pretty sure I have this figured out, will push to develop shortly.

[phatboyg]

Please test once it lands on develop, and verify the behavior. Feel free to open it again if the problem continues (or moves).